fac26 / code-reviews

0 stars 3 forks source link

Code review for Dominic, Georgia and Derek. #5

Open ko-karol opened 1 year ago

ko-karol commented 1 year ago

Also created on their repo.

Readme

As of now, the readme is clean and concise.

User Stories

So far, adding a task does not work on the github deployment. But it works if on a local deployment, inside the javascript-functionality branch. It logs the current state of an array with your list items, which is nice.

So far, a user is able to create a new task, and delete a new task.

Areas of improvement and comments

Maybe instead of logging a whole array, log only the new item. There is no clear indication that you can delete a task by clicking on the list item.

File structure looks perfectly fine, aside from one minor detail.

Code Flow

The flow of the code seems a bit weird.

list.addEventListener("click", (x) => {
  let idSome = x.target.getAttribute("data-id");
  if (!idSome) return; // user clicked in something else
  // passing id to function
  taskFromDOM(idSome);
  taskFromArray(idSome);
});

Seems to be calling functions from below in the code flow. Wouldn't it make more sense to create those functions before you need to use them?

Readability

Code is a little bit hard to understand at this point. Maybe taskTo and taskFrom function could be condensed into one? Not sure.