Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor usages of $scope in MainCtrl and HMTL #22

Open
ThomasBurleson opened this issue Apr 17, 2014 · 2 comments
Open

Poor usages of $scope in MainCtrl and HMTL #22

ThomasBurleson opened this issue Apr 17, 2014 · 2 comments

Comments

@ThomasBurleson
Copy link

Consider the code:

myModule.controller('MainCtrl', function ($scope, AngelloModel, AngelloHelper) {
    $scope.currentStory = null;
    $scope.currentStatus = null;
    $scope.currentType = null;
    $scope.editedStory = {};

    $scope.types = AngelloModel.getTypes();
    $scope.statuses = AngelloModel.getStatuses();
    $scope.stories = AngelloModel.getStories();
    $scope.typesIndex = AngelloHelper.buildIndex($scope.types, 'name');
    $scope.statusesIndex = AngelloHelper.buildIndex($scope.statuses, 'name');

    $scope.setCurrentStory = function (story) {
        $scope.currentStory = story;
        $scope.editedStory = angular.copy($scope.currentStory);

        $scope.currentStatus = $scope.statusesIndex[story.status];
        $scope.currentType = $scope.typesIndex[story.type];
    };

    $scope.createStory = function () {
        AngelloModel.createStory($scope.editedStory);
        $scope.resetForm();
    };

    $scope.setCurrentStatus = function (status) {
        $scope.editedStory.status = status.name;
    };

    $scope.setCurrentType = function (type) {
        $scope.editedStory.type = type.name;
    };

});
@ThomasBurleson
Copy link
Author

It is always a bad idea to use your $scope as a model container and manipulate the data using $scope references.
Instead create separate models and publish those to the $scope.

  1. Use Controller as vm syntax to enforce concept that UI binds to a vm (view model)
  2. Create a contain object for all the current information.
  3. $scope assignments are immediate and understandable

See

myModule.controller('MainCtrl', function( models, utils ) 
{
    var statusIndices = utils.buildIndex(models.statuses, 'name'),
        typeIndices   = utils.buildIndex(models.types   , 'name'),
        vm = this;

    vm.current          = models.current;       // Data
    vm.stories          = models.stories;
    vm.types            = models.types;
    vm.statuses         = models.statuses;

    vm.select            = onSetAsCurrent;         // Event Handlers
    vm.createStory      = onCreateStory;
    vm.setCurrentStatus = onSetCurrentStatus;
    vm.setCurrentType   = onSetType;


    // *************************************
    // Private Event Handlers
    // *************************************

    function onSetAsCurrent(story) 
    {
        var current = models.current;

        current.story  = story;

        current.status = statusIndices[ story.status ];
        current.type   = typeIndices[ story.type ];
    };

    function onCreateStory() 
    {
        models.createStory($scope.editedStory);
        resetForm();
    };

    function onSetCurrentStatus( status, current ) 
    {
        current = current || models.current;
        if ( current  ) current.status = status.name;

    };

    function onSetType(type, current) 
    {
        current = current || models.current;
        if (  current ) current.type = type.name;
    };

});

@ThomasBurleson
Copy link
Author

Additionally - with the use of Controller as syntax, your HTML is considerable more easy to understand:

<div ng-controller="MainCtrl as vm">
        <div class="span4 sidebar-content">
            <h2>Stories</h2>
            <div class="story" ng-repeat="story in vm.stories" ng-click="vm.select(story)">
                <h4>{{story.title}}</h4>
                <p>{{story.description}}</p>
            </div>
            <br>
            <a class="btn" ng-click="vm.createStory()" ><i class="icon-plus"></i></a>
        </div>
</div>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant