Michael
Michael

Reputation: 140

More efficient way of dealing with my functions in javascript

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?

Any other ideas are most welcome offcourse.

Upvotes: 0

Views: 57

Answers (2)

fafl
fafl

Reputation: 7387

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);
};

Upvotes: 0

JavaKungFu
JavaKungFu

Reputation: 1314

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);
};

Upvotes: 1

Related Questions