Skip to content

expected the task#1703

Open
SerhiyShimko wants to merge 1 commit intomate-academy:masterfrom
SerhiyShimko:develop
Open

expected the task#1703
SerhiyShimko wants to merge 1 commit intomate-academy:masterfrom
SerhiyShimko:develop

Conversation

@SerhiyShimko
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

I am approving your solution because it correctly implements createTree(element, data) with recursion and produces the required nested <ul>/<li> structure for the provided food object, fulfilling the core task requirements. The main function works as expected for normal input and there are no critical blockers that would prevent the task from being considered complete.

There are, however, a few robustness improvements worth considering:

  • In src/scripts/main.js, avoid creating and appending an empty <ul> when data is an empty object by checking Object.keys(data).length > 0 before creating the list.
  • Add a defensive check that element is actually a DOM element (for example, if (!(element instanceof Element)) throw new Error('element must be a DOM Element')) so invalid calls fail early with a clear message.
  • When recursing, guard against null and unintended arrays by using if (data[key] && typeof data[key] === 'object' && !Array.isArray(data[key])) before calling createTree again.

These are not blockers for this assignment, but addressing them will make your code more robust and easier to maintain in real-world scenarios. Nicely done on implementing the recursive structure and satisfying the task’s main functional requirements.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example


function createTree(element, data) {
// WRITE YOUR CODE HERE
const ul = document.createElement('ul');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You create the <ul> unconditionally. This can append an empty <ul> when data is an empty object. Consider creating/appending the <ul> only when Object.keys(data).length > 0 to avoid empty lists.

// WRITE YOUR CODE HERE
const ul = document.createElement('ul');

element.append(ul);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

element.append(ul) assumes element is a DOM Element. The task requires element to be a DOM element — consider validating it (for example if (!(element instanceof Element)) throw new Error('element must be a DOM Element')) to fail early with a clear message.

ul.append(newLi);
newLi.textContent = key;

if (typeof data[key] === 'object' && Object.keys(data[key]).length > 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition typeof data[key] === 'object' is true for null (and for arrays). This can cause errors when data[key] is null. Use a safer check such as data[key] && typeof data[key] === 'object' and, if arrays are not intended, !Array.isArray(data[key]) before recursing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants