Simple Todo app

Question

I'm writing a todo app as a test. Having never done Ionic nor Angular apps before, I am not sure if I am following best practices here.

What I have done is try to keep my controllers thin by placing all persistant logic in the model (service?). And instead of hard coding links in the view, I am calling a function in the controller.

Are there any other things I should or shouldn't be doing?

// controllers

angular.module('starter.controllers', [])

.controller('TodosCtrl', function($scope, Todos, $state) {

  $scope.todos = Todos.all();

  $scope.data = {
    showDelete: false
  };

  $scope.add = function() {
    $state.go('tab.add');
  };

  $scope.remove = function(todo) {
    Todos.remove(todo);
  };
})

.controller('TodoDetailCtrl', function($scope, $stateParams, Todos, $state) {

  $scope.todo = Todos.get($stateParams.todoId);

  $scope.remove = function(todo) {
    Todos.remove(todo);
    $state.go('tab.todos');
  };
})

.controller('TodoAddCtrl', function ($scope, $state, Todos) {
  $scope.content = {};

  $scope.add = function() {

    var todo = $scope.content.name;

    if(todo) {

      $scope.content.name = '';

      Todos.add({
        id: Todos.all().length + 1,
        name: todo
      });

      $state.go('tab.todos');
    }
  };

})

.controller('AccountCtrl', function($scope) {
  // unused for now
  $scope.settings = {
    enableServer: true
  };
})



// services

angular.module('starter.services', ['ngStorage'])

.factory ('StorageService', function ($localStorage) {

  $localStorage = $localStorage.$default({
    todos: []
  });

  var _getAll = function () {
    return $localStorage.todos;
  };
  var _add = function (todo) {
    $localStorage.todos.push(todo);
  }
  var _remove = function (todo) {
    $localStorage.todos.splice($localStorage.todos.indexOf(todo), 1);
  }
  return {
    getAll: _getAll,
    add: _add,
    remove: _remove
  };
})

.factory('Todos', function(StorageService) {
  // Might use a resource here that returns a JSON array
  var todos = StorageService.getAll();

  return {
    all: function() {
      return todos;
    },
    remove: function(todo) {
      StorageService.remove(todo);
    },
    add: function(todo) {
      StorageService.add(todo);
    },
    get: function(todoId) {
      for (var i = 0; i < todos.length; i++) {
        if (todos[i].id === parseInt(todoId)) {
          return todos[i];
        }
      }
      return null;
    }
  };
})

Answer 1

For Angular Best Practices (or very good practice) take a look at John Papa's Style Guide: https://github.com/johnpapa/angular-styleguide/blob/master/a1/README.md

It is well written, easy to understand, includes examples and really helps in development!

---

Now to your code.

Files

In an actual application, you would put every single module, controller, service, and directive into a seperate file and name it accordingly. Basically, everytime you write angular.module, angular.controller, [...], it's a new file! This might seem ridiculously at first, but in a big project, it really helps to find stuff. A build chain, e.g. Gulp, will later put everything back together and even optimize it.

Controller vs Services

You want the controller to have anything regarding the view. It includes callbacks (for ng-clicks e.g.) and maybe some logic that is very specific to the view. In the end, it is just a controller and just controlls what should happen and what the view should be able to use. Use a service/factory for all the real application logic. Everything that might be reusable gets a own service. Get data from the server? Service. Calculate something based on the user input? If it might be used somewhere else too, create a service! (GreatService.calculate(userInput)) This way you not only can use it again, but it also really helps you to test your logic in unit tests.

In your specific Todo Example: I think you might have used too many controllers here. I dont's see your template, but this would be probably just fine with one controller and your service. Unless this has multiply states?

Take a look at John Papa's style guide, it's worth ! By the way, using $scope is not recommended. It does work, but is still a thing from the beginnings of Angular. Check this out: https://johnpapa.net/do-you-like-your-angular-controllers-with-or-without-sugar/