current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 1 down vote favorite
1

I am relatively new to Angular and need to put together an application to update / save / display values. The code is working, but I wanted your input the make sure that I am doing this 'the angular way'.

JavaScript

var app = angular.module('store', []);

app.controller('TableController', ['$scope', '$log',function($scope, $log){
   $scope.family = familyMembers;


   $scope.SaveMember = function() {
       var saveMember = { name: $scope.name, age: $scope.age };
       if ($scope.editItem == -1) {
            $scope.family.push(saveMember);
            // insert in db here
       }
       else {
           $scope.family[$scope.editItem] = saveMember;
           $scope.editItem = -1;
           // update db here
       }       
       Init();
   };

   $scope.DeleteMember = function(idx) {
       $scope.family.splice(idx,1);
       // delete from db here
   }

   $scope.EditMember = function(idx){
       $scope.name = $scope.family[idx].name;
       $scope.age = $scope.family[idx].age;
       $scope.editItem = idx;
       $scope.buttonText = 'Update';
   }

   function Init() {
        $scope.name = '';
        $scope.age = '';
        $scope.editItem = -1;
        $scope.buttonText = 'Add new';
   }

   Init();

}]);

var familyMembers = [
    { name: "Joe", age: 42}
    , { name: "Jane", age: 40}
    , { name: "Bob", age: 13}
];

HTML

<html ng-app="family">
    <head>
        <link href="./css/bootstrap.min.css" rel="stylesheet" type="text/css" />
        <style>
            .ng-valid.ng-dirty {border-color: green}
            .ng-invalid.ng-dirty{border-color: red}
            table tr td {padding: 4px}
            [ng-click] {cursor: pointer}
        </style>
    </head>
    <body>
        <div ng-controller="FamilyController" style="margin: 40px">
            <h1>Family Members:</h1>
            <table>
                <tr ng-repeat="p in family track by $index">
                    <td ng-click="EditMember($index)">{{p.name}}</td>
                    <td>{{p.age}}</td>
                    <td><a href ng-click="DeleteMember($index)">X</a></td>
                </tr>
            </table>
            <hr>
            <h1>Add member</h1>
            name <br>
            <input type="text" ng-model="name"><br>
            age <br>
            <input type="number" ng-model="age"><br><br>
            <input type="button" value="{{buttonText}}" ng-click="SaveMember()" />            
        </div>
        <br><br>
    </body>
    <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.3.15/angular.min.js"></script>
    <script src="./js/app.js"></script>
</html>
share|improve this question
up vote 1 down vote accepted

Yup, looks like The Angular Way™. A good rule of thumb for controllers is that 1) it exposes data, 2) it exposes functionality, 3) not tied to any specific template. There's a few things though.

var familyMembers = [
    { name: "Joe", age: 42}
    , { name: "Jane", age: 40}
    , { name: "Bob", age: 13}
];

Why is this outside the controller? Why not define it inside? If you want it defined separately from the controller, consider using a provider/service/factory to store it. Then pull that provider/service/factory into the controller as a dependency.

Init isn't really some initialization. It's resetting your form data. A better way to do this, instead of emptying the object values, is to create a function that returns a blank version of the object and replace the form data.

function createMember(){
  return { name: '', age: '' };
}

function resetForm(){
  $scope.formData = createMember();
}

// Assuming the form uses `$scope.formData` as the form object

As for saving, before you reset, you just copy the form data using angular.copy and pop it in the members array. This way, the object you put in the array isn't the same object referenced by the form data, which may still be subject to binding. Then you reset the form data.

function saveForm(){
  var newMember = angular.copy($scope.formData);
  members.push(newMember);
  resetForm();
}
share|improve this answer
    
Thanks. That was helpful. – Olivier De Meulder Jan 29 at 19:43

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.