More efficient way of dealing with my functions in javascript

Question

So I got these two functions with an if statement which does the same thing.

Which is to check if the value of the input field is empty. if(newTask.value === '')

var newTask = document.getElementById("new-task");

newTask.addEventListener("keyup", function(event) {
    event.preventDefault();
    if(event.keyCode == 13){
        if(newTask.value === ''){
            alert('Fill in a task.');
        } else{
            addTask(newTask.value);
        }
    }
});

newTaskBtn.onclick = function() {
    if(newTask.value === ''){
        alert('Fill in a task.');
    } else{
        addTask(newTask.value);
    }
};

What is the recommended way to make my code more efficient?

Writing another function which does only the checking.
Writing a function within a function?

Any other ideas are most welcome offcourse.

You could move the check for an empty value inside the "addTask" function — fafl
– fafl, Commented May 18, 2017 at 8:54

JavaKungFu · Accepted Answer · 2017-05-17 20:59:16Z

1

You can write a function that does the task checking. For example:

var newTask = document.getElementById("new-task");
function checkTask(taskValue) {
 if(taskValue === ''){
    alert('Fill in a task.');
  } else{
    addTask(taskValue);
  }
}
newTask.addEventListener("keyup", function(event) {
    event.preventDefault();
    if(event.keyCode == 13){
       checkTask(newTask.value);
    }
});

newTaskBtn.onclick = function() {
   checkTask(newTask.value);
};

answered May 17, 2017 at 20:59

JavaKungFu

1,3142 gold badges12 silver badges25 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

fafl · Accepted Answer · 2017-05-17 21:02:15Z

Your code is fine. There is some duplication, but both functions have different preconditions because the keyup has to check the keycode. Adding another level of function calls will make the code less readable imho. I'd just change the structure to use early returns, like this:

var newTask = document.getElementById("new-task");

newTask.addEventListener("keyup", function(event) {
    event.preventDefault();
    if (event.keyCode !== 13) {
        return;
    }
    if (newTask.value === '') {
        alert('Fill in a task.');
        return;
    }
    addTask(newTask.value);
});

newTaskBtn.onclick = function() {
    if (newTask.value === '') {
        alert('Fill in a task.');
        return;
    }
    addTask(newTask.value);
};

Collectives™ on Stack Overflow

More efficient way of dealing with my functions in javascript

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related