0
\$\begingroup\$

I decided to write a simple runnable example to try and express my concern:

index.html

<section ng-app="App" ng-clock>
  <div ng-controller="ShopController as shop">                
      <prize-directive ng-repeat="prize in prizes"></prize-directive>
  </div>
</section>

Inside my shopController I have set a static prize list just for the purpose of keeping it simple.

ShopController.js

angular.module('app.controllers').controller('ShopController',
   ['PrizeService', '$scope', function (PrizeService, $scope) {

    $scope.prizes = [
      {name: 'First Prize', cost: 10, 
         image: 'upload/prize/image1.png', note: 'This is a note'},
      {name: 'Second Prize', cost: 20,
         image: 'upload/prize/image2.png', note: 'This is a note'},
      {name: 'Third Prize', cost: 30, 
         image: 'upload/prize/image3.png', note: 'This is a note'}
    ];

    $scope.reclaim = function (prize) {
      console.log(prize);
    };
}]);

In the original code I use the PrizeService to load a list of available prizes, but back to my main problem:

PrizeDirective

angular.module('app.directives').directive('prizeDirective', function () {
  return {    
    restrict: 'E',
    templateUrl: 'js/directives/PrizeDirective.html'
  };
});

HTML:

<div>    
    <img ng-src="{{Endpoint + prize.image}}" />
    <h3>{{prize.name}}</h3>
    <span>Pontos: {{prize.cost}}</span>    
    <p>{{prize.note}}</p>
</div>
<reclaim-directive></reclaim-directive>

Here in the HTML portion of the directive, I'm using a variable name that was defined in the index.html. Although it bothers me, I'm willing to let it go because the variable name was defined inside the ng-repeat code, which is inside the prize-directive directive. But when we go to the reclaim-directive, things get a little bit more heated:

angular.module('app.directives').directive('reclaimDirective', function () {
  return {    
    restrict: 'E',
    template: '<button ng-click="reclaim(prize)">Resgatar</button>'
  };
});

Note: I'm actually using the templateUrl here, but again, I'm simplifying it just to show my point.

Now I'm using a function name defined inside ShopController in the Reclaim Directive. Differently from OOP, this is not really an inheritance of an abstract method which even the IDE can tell you that something is wrong and, to me, it seems like a bad practice.

I would love to hear people's thoughts on this approach and if there is anything that I'm missing that could help clean up this code.

\$\endgroup\$
2
\$\begingroup\$

Now I'm using a function name defined inside ShopController in the Reclaim Directive... it seems like a bad practice.

Yep, this kind of code will be hard to debug and maintain.

  • You're depending on the existence of reclaim to be somewhere in the scope chain, on some controller enclosing your directive.
  • There's no indication whenever you use the directive that reclaim is missing in the scope. The directive will fail only if you click it.
  • Without proper tools like Batarang, in a highly nested controller structure, reclaim will be hard to find when you don't know which controller it resides.

If <reclaim-directive> isn't that huge, and is not used anywhere else independently, then it's contents should belong to <prize-directive>.

If <reclaim-directive> is big and independent enough to be a separate directive, then consider making it a sibling of <reclaim-directive>. That way, you avoid the extra nested level:

<div ng-controller="ShopController as shop">                
  <prize-directive ng-repeat="prize in prizes"></prize-directive>
  <reclaim-directive></reclaim-directive>
</div>

Most importantly, instead of depending on implicit existence of reclaim in the scope, why not provide it to the directive explicitly? Construct a scope for the directive, which accepts a binding of reclaim. I believe this is the way to do it:

<prize-directive ng-repeat="prize in prizes" on-reclaim="reclaim"></prize-directive>

angular.module('app.directives').directive('prizeDirective', function () {
  return {
    restrict: 'E',
    templateUrl: 'js/directives/PrizeDirective.html',
    scope: {
      onReclaim: '='
    }
  };
});

<!-- onReclaim assigned to on-reclaim comes from the prizeDirective -->
<reclaim-directive on-reclaim="onReclaim"></reclaim-directive>

angular.module('app.directives').directive('reclaimDirective', function () {
  return {    
    restrict: 'E',
    template: '<button ng-click="onReclaim(prize)">Resgatar</button>',
    scope: {
      onReclaim: '='
    }
  };
});
\$\endgroup\$
  • \$\begingroup\$ I ran into a problem in making reclaim-directive a sibling of prize-directive because the ng-repeat clause is at the prize-directive, which makes the reclaim-directive show up only once at the end (not repeat through). \$\endgroup\$ – Marco Aurélio Deleu Nov 7 '15 at 15:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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