2
\$\begingroup\$

I have built a To Do App in Javascript, using Webpack and some modules. The app lets you store projects (School, Sport, Grocery, etc.)... And inside these projects, you can store todo items... You can also edit the todo item, and click it do delete/finish it.

The app should be built with OOP principles in mind, and that's the main concern with the code, and the reason that I want it to be reviewed. This is the weakness of my code. And I would want you to give me tips on how to improve this...

It is my first post here... I hope I followed all the rules. And that is why I posted only the following code here, and shared my github repo if there is a person generous to look into it.

Github repo! (Posted it because of webpack...)

Index.js:

if (myProjects.length == 0) {
  defaultProject();
}
//defaultProject();
// Add new project!

newProjectListener.addEventListener("submit", (event) => {
  event.preventDefault();
  const newProjectTitle = document.getElementById("newProjectName").value;

  if (newProjectTitle == "") {
  } else {
    newProjectEvent(event);
    addProjectUI(newProject);
   // saveToLocalStorage(myProjects);
    emptyForm();
  }
});

// Delete project, adding event listeners to all future trash buttons for projects...
window.addEventListener("click", (event) => {
  let element = event.target.classList.contains("bi-trash")
    ? event.target.parentElement
    : event.target.classList.contains("trash-project")
    ? event.target
    : false;
  if (element) {
    let itemToRemove = element.parentElement.parentElement;
    deleteProject(itemToRemove);
    deleteItemUI(itemToRemove);
    cleanToDoView();
    // localStorage.clear();
    // saveToLocalStorage(myProjects);
  }
});

// clicked project

projectListDiv.addEventListener("click", (event) => {
  if (event.target.tagName == "P") {
    resetClickedProject();
    clickedProject(event);
    clickedProjectIndex = idClickedProject(event);
    cleanToDoView();
    render();
  }
});

// new to do...
newToDoListener.addEventListener("submit", (event) => {
  event.preventDefault();
  if (
    toDoTitle.value == "" ||
    description.value == "" ||
    dueDate.value == "" ||
    priority.value == "" ||
    note.value == ""
  ) {
  } else {
    newToDoEvent(event, clickedProjectIndex);
    let toDo = newToDo;
    appendToDo(toDo);
    // localStorage.clear();
    // saveToLocalStorage(myProjects);
    emptyToDoForm();
  }
});

tableListener.addEventListener("click", (event) => {
  let element = event.target.classList.contains("delete")
    ? event.target.parentElement.parentElement
    : event.target.classList.contains("fa-check")
    ? event.target.parentElement.parentElement.parentElement
    : false;
  if (element) {
    let deleteItem = element;
    deleteToDoFromObject(deleteItem, clickedProjectIndex);
    deletToDoUI(deleteItem);
    // localStorage.clear();
    // saveToLocalStorage(myProjects);
  }
});

window.addEventListener("click", (event) => {
  let element = event.target.classList.contains("edit-button")
    ? event.target.parentElement.parentElement
    : event.target.classList.contains("fa-pencil-square-o")
    ? event.target.parentElement.parentElement.parentElement
    : false;
  if (element) {
    toDoIndex = clickedToDoIndex(event, element);
    editTodo(clickedProjectIndex, toDoIndex);
  }
});

editToDo.addEventListener("submit", (event) => {
  event.preventDefault();
  editFinish(clickedProjectIndex, toDoIndex);
  cleanToDoView();
  render();
  // localStorage.clear();
  // saveToLocalStorage(myProjects);
});

const render = () => {
  myProjects[clickedProjectIndex].toDos.forEach((todo) => {
    appendToDo(todo);
  });
};

const initialLoad = () => {
  myProjects.forEach((project) => {
    addProjectUI(project);
    console.log(project)
    const projectHeader = document.querySelector(".projectName");
    projectHeader.textContent = project.title
  });
};
initialLoad();

const initalTodoLoad = () => {
  myProjects[0].toDos.forEach((todo) => {
    appendToDo(todo);
  });
};
initalTodoLoad();

projectFactory.js:

let myProjects = [];
let newProject;
// let myProjects = localStorage.getItem("projects")
//   ? JSON.parse(localStorage.getItem("projects"))
//   : [
    
//   ];


const saveToLocalStorage = () => {
  localStorage.setItem("projects", JSON.stringify(myProjects));
};


// Project factory, which takes in title and makes toDo array, to which the toDos will be added...
const newProjectFactory = (id, title) => {
  const toDos = [];

  const add_toDo = (toDo) => {
    toDos.push(toDo);
  };

  return { id, title, toDos, add_toDo };
};

const newProjectEvent = (event) => {
  // DOM elements of form ...
  event.preventDefault();
  const newProjectTitle = document.getElementById("newProjectName").value;

  let ID;
  if (myProjects.length > 0) {
    ID = myProjects[myProjects.length - 1].id + 1;
  } else {
    ID = 0;
  }

  newProject = newProjectFactory(ID, newProjectTitle);

  myProjects.push(newProject);
  
};
\$\endgroup\$
2
\$\begingroup\$

This review will focus on what object oriented programming [oop] is and how we can use two concepts of oop (encalpsulation and abstraction) in your code to make it more object oriented.

The self-drawn pictures of this review come from my Github-Repository for a presentation that is supposed to show the basics of oop.

How to write Object-Oriented Code

Object-Oriented Programming is based on the following 4 concepts:

Not all of these concepts have to be implemented per file, class, object, ... in order to be object-oriented.

But following the concepts of abstraction and encapsulation especially at the beginning will make the code much more object-oriented.

To make these concepts easier to remember, there is the acronym called "a pie".

image, that lists the four concepts of oop and a drawing of a pie

Encapsulation

What it is

Encapsulation is used to hide the values or state of a structured data object inside a class, preventing unauthorized parties' direct access to them.

wikipedia

image, that describes encapsulation with the sentence "It is the hiding of the internals." and an drawing of a lock.

What is against the concept of encapsulation?

The variable myProjects has been defined globally and can be read and modified by anyone and everyone.

// in projectFactory.js
let myProjects = [];

// in index.js
if (myProjects.length == 0) { /* ... */ }
//...
const newProjectEvent = (event) => {
  //...
  if (myProjects.length > 0) {/ * ... */}
  // ...
  myProjects.push(newProject);  
};

Breaking the code is easy. All that has to be done is to assign a different value to myProjects:

myProjects = "";

Now you will think that nobody would do that, but when you work in a team it goes faster than expected. Even if it is accidental.


The same for toDos of you project object:

myProjects[clickedProjectIndex].toDos.forEach((todo) => {
 appendToDo(todo);
});

The variable toDos can be intervened again without restriction and we could modify it again: myProjects[clickedProjectIndex].toDos = /* something wrong */

How can we fix the violations?

We can hide the variables inside objects with #-Symbol.

class ProjectCollection {
  #projects = [];
}

const projects = new ProjectCollection();
// not possible anymore
projects.projects = /* something wrong */ 

Abstraction

What it is

The term encapsulation refers to the hiding of state details, but extending the [...] associate behavior most strongly with the data, and standardizing the way that different data types interact, is the beginning of abstraction.

wikipedia

What is against the concept of abstraction?

Since the code has no encapsulation, there is no abstraction. For example, we ask whether the length of the array myProjects is 0:

if (myProjects.length == 0) {
 // ...
}

However, we can abstract this by calling a method:

if (myProjects.containsNon()) {
  // ...
}
const myProjects = new ProjectCollection();

class ProjectCollection {
  #projects = [];

  containsNon() {
    return this.#projects == 0;
  }
}
\$\endgroup\$
3
  • \$\begingroup\$ Okey, will keep in mind. So basically also modules (IIFE) should be some kind of object-oriented programming. As they help you encapsulate and hide the inner parts of code, and therefore cannot be accessed from the outside? \$\endgroup\$ – Žiga Grošelj Oct 1 '20 at 19:28
  • 1
    \$\begingroup\$ Important thing to note about private class fields: “Both Public and private field declarations are an experimental feature (stage 3) proposed at TC39, the JavaScript standards committee. Support in browsers is limited, but the feature can be used through a build step with systems like Babel. See the compat information below. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Oct 2 '20 at 2:44
  • 1
    \$\begingroup\$ @ŽigaGrošelj, yes, these concepts can also be followed with IIFE. :) \$\endgroup\$ – Roman Oct 2 '20 at 4:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.