Reputation: 140
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
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
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