-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Delete node function for tree #6
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for parths-algorithms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ Changes requested. Reviewed everything up to a598594 in 51 seconds
More details
- Looked at
288
lines of code in10
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_qCFbzkCfKuIYEudJ
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
@@ -0,0 +1,154 @@ | |||
import { root } from "postcss" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import of root
from postcss
appears to be unused and should be removed to keep the code clean and relevant.
if (root.leftNode && root.rightNode) { | ||
const nextNode = findNextNode(root.rightNode) | ||
const tempValue = nextNode.value | ||
deleteNode({ value: nextNode.value, root: root }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recursive deletion in deleteNode
function might cause issues if the node to delete is the root itself. Consider handling the case where the node to delete is the root to prevent potential infinite recursion or incorrect tree manipulation.
|
||
|
||
|
||
const findNextNode = (root: Node) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The findNextNode
function incorrectly assumes the next node is always the left-most child. This does not account for the right children, which should be considered when finding the next node in in-order traversal. Consider revising this function to correctly handle finding the next node.
</div> | ||
<div className="flex flex-col underline items-center"> | ||
|
||
<Link href="/algorithms/bubblesort"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The links in this file still point to /algorithms/...
paths, but should be updated to /dsa/...
to reflect the new directory structure and ensure they lead to the correct pages.
Summary:
Reorganized project structure, added tree operations, and updated redirection path.
Key points:
src/app/algorithms/page.tsx
.src/app/algorithms/bubblesort/bubbleSort.ts
tosrc/app/dsa/bubblesort/bubbleSort.ts
.src/app/algorithms/bubblesort/page.tsx
tosrc/app/dsa/bubblesort/page.tsx
.src/app/algorithms/insertionsort/insertionSort.ts
tosrc/app/dsa/insertionsort/insertionSort.ts
.src/app/algorithms/insertionsort/page.tsx
tosrc/app/dsa/insertionsort/page.tsx
.src/app/algorithms/quicksort/page.tsx
tosrc/app/dsa/quicksort/page.tsx
.src/app/algorithms/quicksort/quicksort.ts
tosrc/app/dsa/quicksort/quicksort.ts
.src/app/dsa/tree/tree.ts
withinsertNode
,readTree
, anddeleteNode
functions.src/app/page.tsx
to redirect to/dsa
instead of/algorithms
.Generated with ❤️ by ellipsis.dev