2
\$\begingroup\$

I use a lot of for loops and I think it slows the code down a lot but I have no idea how else this code could be written. Its for a to-do list app and each for loop adds an eventListener to each button on each task (edit, delete, check).

Is there a more efficient way to write this?

import {myTasks,newTaskForm,rightSide,projectContainer,displayType} from './index';

const today = new Date().toLocaleDateString('en-ca');
let count = 0;
let todayTasks = [];

const tasks = () => {
  const task = (title, duedate, completed, position, editing, checked, display, id, taskClass) => {
    return {title,duedate,completed,position,editing,checked,display,id,taskClass}
  }

  const addTask = () => {
    const newTask = task(`<input type="text" id="edit-title-${count}" placeholder="Task" required>`,
    `<input type="date" id="edit-date-${count}" name="duedate" min="${today}">`, false, count, false);

    if (newTask.completed == true) {
      newTask.checked = "checked";
      newTask.taskClass = "completed-task"
    }
    else if (newTask.completed == false) {
      newTask.checked = ""
      newTask.taskClass = "task-container"
    }

    if (displayType == "today") {
      newTask.duedate = `<input type="date" id="edit-date-${count}" name="duedate" min="${today}" value="${today}">`;
    }

    myTasks.push(newTask);

    newTask.id = "task-container-" + count;
    count += 1;
    updateTaskDisplay();
  }

  const updateTaskDisplay = () => {
    for (var i=0;i<myTasks.length;i++) {
      myTasks[i].display = "<div class='" + myTasks[i].taskClass + "' id='" + myTasks[i].id + "'>" + "<div id='newtask-completed'>" + "<input type='checkbox' id='toggle'" + myTasks[i].checked + ">"
      + myTasks[i].title + "</div>" + "<div id='newtask-duedate'>" + myTasks[i].duedate
      + "</div>" + "<div class='buttons'>" + "<button id='edit'>" + `<img src="edit-button.svg" alt="Edit" height="12px">` + "</button>" + "<button id='delete'>" + 'x' + "</button>" + "</div>" + "</div>"
    }
    if (displayType == "today") {
      displayTodayTasks();}

    else if (displayType == "all") {
      displayAllTasks();}
  }

  const displayAllTasks = () => {
    rightSide.innerHTML = "";
    for (var i=0;i<myTasks.length;i++) {
      rightSide.innerHTML += myTasks[i].display
    }
    toggleCheck();
    editTask();
    removeTask();
  }

  const displayTodayTasks = () => {
    rightSide.innerHTML = "";
    todayTasks = [];
    for (var i=0;i<myTasks.length;i++) {
      if (myTasks[i].duedate == today || myTasks[i].duedate.includes(`value="${today}"`)) {
        todayTasks.push(myTasks[i])
        rightSide.innerHTML += myTasks[i].display
    }}
    toggleCheck();
    editTask();
    removeTask();
  }

  const toggleCheck = () => {
    const toggle = document.querySelectorAll('#toggle')
    let toggleIndex;
    toggle.forEach((button, i) => {
      button.addEventListener("click", () => {
        if (displayType == "today") {
          toggleIndex = myTasks.indexOf(todayTasks[i]);
        }
        else if (displayType == "all") {
          toggleIndex = i;
        }
        myTasks[toggleIndex].completed = !myTasks[toggleIndex].completed
        if (myTasks[toggleIndex].checked == "checked") {
          myTasks[toggleIndex].checked = "";
          myTasks[toggleIndex].taskClass = "task-container"
          updateTaskDisplay();
        }
        else {
          myTasks[toggleIndex].checked = "checked";
          myTasks[toggleIndex].taskClass = "completed-task"
          updateTaskDisplay();}
    })})}

  const editTask = () => {
    const edit = document.querySelectorAll("#edit")
    let editIndex;
    edit.forEach((editButton, i) => {
      editButton.addEventListener("click", () => {
        if (displayType == "today") {
          editIndex = myTasks.indexOf(todayTasks[i]);
        }
        else if (displayType == "all") {
          editIndex = i;
        }
        if (myTasks[editIndex].editing) {
          myTasks[editIndex].title = `<input type='text' id='edit-title-${myTasks[editIndex].position}' size='15' value="${myTasks[editIndex].title}">`
          myTasks[editIndex].duedate = `<input type='date' id='edit-date-${myTasks[editIndex].position}' min=${today} value="${myTasks[editIndex].duedate}">`
          updateTaskDisplay();
          myTasks[editIndex].editing = false;
        }
        else if (!myTasks[editIndex].editing) {
          let editTitle = document.getElementById(`edit-title-${myTasks[editIndex].position}`)
          let editDate = document.getElementById(`edit-date-${myTasks[editIndex].position}`)
          myTasks[editIndex].title = editTitle.value;
          myTasks[editIndex].duedate = editDate.value;
          updateTaskDisplay();
          myTasks[editIndex].editing = true;
        }
      })
  })}

  const removeTask = () => {
    const del = document.querySelectorAll("#delete")
    del.forEach((x, i) => {
      x.addEventListener("click", () => {
        if (displayType=="all") {
          myTasks.splice(i, 1)
          displayAllTasks()}
        else if (displayType=="today"){
          myTasks.splice((myTasks.indexOf(todayTasks[i])), 1)
          todayTasks.splice(i, 1)
          displayTodayTasks()}
    })})}

  return {tasks, task, addTask, displayAllTasks, displayTodayTasks}
}

export {tasks, today};

\$\endgroup\$
9
  • 1
    \$\begingroup\$ I strongly recommend running your code through an autoformatter such as Prettier. The Lisp-style braces are very unusual. The word "efficient" is very loaded and can mean a lot of things in programming. It's a good idea to clarify which "efficient" you mean here. \$\endgroup\$
    – ggorlen
    Commented Sep 20, 2022 at 2:04
  • \$\begingroup\$ I will try that thanks, and however you want to interpret the word efficient is fine by me. I just mean is this well written code or does it run way too slowly and is there a significantly better/faster way to do it? \$\endgroup\$
    – Helan
    Commented Sep 20, 2022 at 20:19
  • \$\begingroup\$ What I mean is that execution speed and code cleanliness/elegance are totally different types of "efficient". Sometimes you need to sacrifice one to get the other. \$\endgroup\$
    – ggorlen
    Commented Sep 20, 2022 at 20:21
  • \$\begingroup\$ oh in that case i'm purely talking about the forEach and for loops i'm using, is that the best way to write this for execution speed or is there a much faster way? \$\endgroup\$
    – Helan
    Commented Sep 20, 2022 at 20:27
  • \$\begingroup\$ Is execution speed really a problem with the app? If it's not meeting your needs, please specify how so (how many todos are you expecting) and provide profiling details if you've profiled the code, as well as any other necessary environment details. Very likely, it doesn't matter much and it's micro-optimization/trivia. \$\endgroup\$
    – ggorlen
    Commented Sep 20, 2022 at 20:30

0

Browse other questions tagged or ask your own question.