# Simple todo application

I have been learning HTML/JavaScript and CSS. I have gone through some of the online tutorials, PDFs and books as well. Though, now I am familiar with syntactical stuff, I wasn't sure how I should design or structure code, so I started with a small ToDo application. I didn't want to use any frameworks. As I am still learning, I thought I should spend sometime learning the basics before using any frameworks.

GitHub

{
(function (){

//Create a modal, which contains an array to hold list of todos
var modal = {
init: function () {
this.todolist = [];
this.todolist = JSON.parse(localStorage.getItem("tododata"));
if (this.todolist === null)
this.todolist = [];
},
getTodoList: function () {
return this.todolist;
},
setTodoState: function (intodoid) {
//Set status
this.todolist[intodoid].status === "closed" ?
this.todolist[intodoid].status = "open" : this.todolist[intodoid].status = "closed";
localStorage.setItem("tododata", JSON.stringify(this.todolist));
},
setNewTodo: function (inval) {
this.todolist.push({
id: this.todolist.length,
name: inval,
status: "open"
});
localStorage.setItem("tododata", JSON.stringify(this.todolist));
this.myFirebaseRef.push({id: this.todolist.length, name: inval, status: "open"});
},
clearall: function () {
this.todolist = [];
}
}

//Create a View, which interacts with Controller and alters the display in HTML
var view = {
init: function () {
this.todolst = $('#todolistelm'); this.todolst.html(' '); view.render();$('#clearall').click(function () {
localStorage.clear();
controller.clearall();
view.render();
});
},
render: function () {
var htmlStr = '';
var clr = true;
var todoelem = $('#todolistelm'); todoelem.html(''); if (controller.getTodoList().length === 0) {$("#cleanmsg").css("visibility", "visible");
$("#displaysection").css("visibility", "hidden") } else {$("#cleanmsg").css("visibility", "hidden");
$("#displaysection").css("visibility", "visible") } controller.getTodoList().forEach(function (todo) { //DOM elements var elemId = ' '; var lielem =$('<li></li>');
var spanelem = $('<span></span>'); var taskid = "#task" + todo.id; //Create <li> element and add required classes lielem.addClass("list-group-item"); //Set status of the todo item todo.status === "closed" ? lielem.addClass("completed") : lielem.addClass(""); //Set alternate color clr === true ? lielem.addClass("colored") : lielem.addClass(""); clr = !clr; //Create <span> and add required classes and IDs spanelem.addClass("glyphicon"); spanelem.addClass("glyphicon-ok pull-right"); spanelem.addClass("todocomplete"); spanelem.attr('id', "task" + todo.id); //Append span and name to <li> lielem.append(spanelem); lielem.append(todo.name); //Append <li> to main list$(todoelem).append(lielem);

$(taskid).click(function (todoid) { return function () { controller.setTodoState(todoid); view.render(); }; }(todo.id)); }); //Set focus on input textbox by default$('#intodo').focus();
}
};

//Create a controller, which acts as a mediator between View and Modal
var controller = {
init: function () {
//Initialize view and modal
modal.init();
view.init();
},
getTodoList: function () {
return modal.getTodoList();
},
setTodoState: function (intodoid) {
modal.setTodoState(intodoid);
},
setNewTodo: function (inval) {
modal.setNewTodo(inval);
},
clearall: function () {
modal.clearall();
}
};

//Set ENTER key on the input text box
$('#intodo').keypress(function (e) { if (((e.which && e.which == 13) || (e.keyCode && e.KeyCode == 13)) &&$("#intodo").val().trim().length > 0) {
controller.setNewTodo($("#intodo").val()); view.render();$("#intodo").val('');
}
});

//Set the controller to start the work
controller.init();
})();
}


Can someone go through this code and let me know if this follows the best practices or not, as well as any areas that I should probably be focusing on?

From a quick once over:

• Your first line creates an anonymous function, it offers you the opportunity to pick a great function name that conveys to the reader what he is about to be exposed to like (function todoApp(){
• This

init: function () {
this.todolist = [];
this.todolist = JSON.parse(localStorage.getItem("tododata"));
if (this.todolist === null)
this.todolist = [];
},


could be

init: function () {
this.todolist = JSON.parse(localStorage.getItem("tododata")) || [];
}


essentially you were overwriting this.todolist = [] with
this.todolist = JSON.parse(localStorage.getItem("tododata"));
furthermore you should never drop the curly braces after an if block

• You are copy pasting this line of code all over the place, you should have a dedicated function for this:
localStorage.setItem("tododata", JSON.stringify(this.todolist));
• This:

setTodoState: function (intodoid) {
//Set status
this.todolist[intodoid].status === "closed" ?
this.todolist[intodoid].status = "open" : this.todolist[intodoid].status = "closed";
localStorage.setItem("tododata", JSON.stringify(this.todolist));
},


has a few problems, intodoid is an unfortunate name because it is not lowerCamelCase and because it smells like Hungarian notation. Also the repetition of this.todolist[intodoid].status is too much. I would suggest something like this:

toggleTodoState: function (todoId) {
//Toggle status
var status = this.todolist[todoId].status;
this.todolist[todoId].status = status == "closed" ? "open" : "closed";
updateTodoData();
},