add task solution#1749
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because there is a functional bug that breaks a core requirement for building the tree structure. In src/scripts/main.js, your createTree function returns early when Object.keys(data).length === 0 (around lines 23–25), which means leaf nodes whose values are {} never get corresponding <li> elements; this violates the requirement that all keys in data appear as list items in the tree ([CHECKLIST ITEM #1] and [CHECKLIST ITEM #2]). To fix this, always create an <li> for each key, and only recurse to build a nested <ul> when the child object has keys; don’t skip the parent <li> just because its child object is empty. A second, smaller robustness issue is that Object.keys(data) assumes data is a non-null object; adding a guard like if (!data || typeof data !== 'object') return; before using Object.keys will prevent runtime errors if createTree is called with invalid input. It would also be safer to iterate with Object.keys(data).forEach(...) or check hasOwnProperty instead of using for...in, to avoid accidentally picking up inherited properties. Aside from these issues, your recursion approach and DOM manipulation are clear and on the right track, so fixing the leaf-node handling will bring your solution in line with the task requirements.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| function createTree(element, data) { | ||
| // WRITE YOUR CODE HERE | ||
| if (!element || Object.keys(data).length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const ul = document.createElement('ul'); | ||
|
|
||
| for (const a in data) { | ||
| const li = document.createElement('li'); | ||
|
|
||
| li.textContent = a; | ||
| createTree(li, data[a]); | ||
| ul.appendChild(li); | ||
| } | ||
|
|
||
| element.appendChild(ul); | ||
| } |
| const tree = document.querySelector('#tree'); | ||
|
|
||
| function createTree(element, data) { | ||
| // WRITE YOUR CODE HERE | ||
| if (!element || Object.keys(data).length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| const ul = document.createElement('ul'); | ||
|
|
||
| for (const a in data) { | ||
| const li = document.createElement('li'); | ||
|
|
||
| li.textContent = a; | ||
| createTree(li, data[a]); | ||
| ul.appendChild(li); | ||
| } | ||
|
|
||
| element.appendChild(ul); | ||
| } | ||
|
|
||
| createTree(tree, food); |
There was a problem hiding this comment.
The element parameter is used as a DOM element (passed tree obtained via document.querySelector('#tree')), satisfying checklist item #3. (lines 21, 41)
| function createTree(element, data) { | ||
| // WRITE YOUR CODE HERE | ||
| if (!element || Object.keys(data).length === 0) { | ||
| return; |
There was a problem hiding this comment.
Edge case: the function returns early when Object.keys(data).length === 0. This prevents creating empty
- for empty objects, which is fine, but be careful: if
data is not an object or is null/undefined this will throw. Consider validating that data is an object (e.g., check data && typeof data === 'object'). This addresses robustness though not required by the task.| for (const a in data) { | ||
| const li = document.createElement('li'); | ||
|
|
||
| li.textContent = a; | ||
| createTree(li, data[a]); | ||
| ul.appendChild(li); | ||
| } |
There was a problem hiding this comment.
Using for...in iterates over enumerable properties from the prototype chain as well. If the data objects are plain objects (as in the task), this is fine. If you want to be strict, use Object.keys(data).forEach(...) or guard with hasOwnProperty. Consider this if unexpected prototype properties may appear.
| function createTree(element, data) { | ||
| // WRITE YOUR CODE HERE | ||
| if (!element || Object.keys(data).length === 0) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Minor behavior note: when element is falsy or data is empty the function returns without modifying element. This matches the simple task requirements, but if element is present and data is empty you may want to leave it unchanged or explicitly clear its contents depending on desired behavior.
2 participants
No description provided.