Angular todo list

Question

I'm not sure whether my code looks good or not:

var app = angular.module('Todolist', ['ngResource', 'xeditable']); app.run(function(editableOptions) { editableOptions.theme = 'bs3'; }); app.controller('TasksCtrl', [ '$scope', 'Task', function($scope, Task) { $scope.user = gon.current_user $scope.updateTitle = function(data, task) { Task.update({ user_id: $scope.user.id, id: task.id, title: data }); }; $scope.updatePriority = function(data, task){ Task.update({ user_id: $scope.user.id, id: task.id, priority: data }) }; $scope.updateDueDate = function(task) { var autoclose = this; $('ul.tasks form input').datepicker({ dateFormat: 'yy-mm-dd', onSelect: function(date, instance) { task.due_date = date; Task.update({ user_id: $scope.user.id, id: task.id }, { due_date: date }); autoclose.$editable.scope.$form.$cancel(); } }); }; $scope.tasks = Task.query({ user_id: $scope.user.id, status: 'incompleted' }); $scope.completedTasks = Task.query({ user_id: $scope.user.id, status: 'completed' }); $scope.addNewTask = function() { var task = Task.save({user_id: $scope.user.id}, ($scope.newText)); $scope.tasks.push(task); $scope.newText = {}; }; $scope.deleteTask = function(task){ if (confirm('Are you sure')) { var index = $scope.tasks.indexOf(task); Task.delete({ user_id: $scope.user.id, id: task.id }, function(success){ $scope.tasks.splice(index, 1); }); } }; $scope.complete = function(task) { Task.update({ user_id: $scope.user.id, id: task.id, task: { completed: true } }, function() { var index; index = $scope.tasks.indexOf(task); $scope.tasks.splice(index, 1); $scope.completedTasks.push(task); }); }; $scope.restore = function(task) { Task.update({ user_id: $scope.user.id, id: task.id, task: { completed: false } }, function() { var index; index = $scope.completedTasks.indexOf(task); $scope.completedTasks.splice(index, 1); $scope.tasks.push(task); }); }; $scope.sort = function(property) { var arrow = $("#" + property); var direction; $('.sort .glyphicon'); arrow.addClass('incompleted'); if (arrow.hasClass('glyphicon-arrow-down')) { arrow.removeClass('glyphicon-arrow-down'); arrow.addClass('glyphicon-arrow-up'); direction = 'desc'; } else { arrow.addClass('glyphicon-arrow-down'); arrow.removeClass('glyphicon-arrow-up'); direction = 'asc'; } $scope.tasks = Task.query({ user_id: $scope.user.id, status: 'incompleted', order_by: property, direction: direction }); }; } ]); 

Answer 1

$scope.user = gon.current_user 

What's gon? I assume it's your authentication/session object with all the things the app needs before running. Best if you put it inside a service or a value so that you can easily use dependency injection with it. That way, your controllers are uniform and only depend on stuff that comes from dependencies, not from extraterrestrial entities like globals.

Task.delete({ user_id: $scope.user.id, id: task.id },function(success){ $scope.tasks.splice(index, 1); }); 

Suggesting you use promises instead of callbacks. They look about the same for simple cases (just a callback passed to then). But promises allow you more control of the data flow. It allows you to modify values, chain successive async operations, wait for multiple parallel promises etc.

var arrow = $("#" + property); var direction; $('.sort .glyphicon'); arrow.addClass('incompleted'); if (arrow.hasClass('glyphicon-arrow-down')) { arrow.removeClass('glyphicon-arrow-down'); arrow.addClass('glyphicon-arrow-up'); direction = 'desc'; } else { arrow.addClass('glyphicon-arrow-down'); arrow.removeClass('glyphicon-arrow-up'); direction = 'asc'; } 

In frameworks like Angular, you only worry about manipulating the data. Normally, you don't do DOM manipulation in Angular anymore except for cases where you have to, like appending your datetime plugin. You could delegate this down as template logic using a scope variable and ng-class:

$scope.isDescending = false; // ascending by default $scope.sort = function(){ $scope.isDescending = !$scope.isDescending; // toggle // derrive direction for Task.query var direction = ['asc', 'desc'][+$scope.isDescending]; } <div ng-class="{ 'glyphicon-arrow-down': isDescending, 'glyphicon-arrow-up': !isDescending }"></div>