Code smell in AngularJS custom directive

Question

In the below code.

<!DOCTYPE html>
<html ng-app="app10" ng-cloak>
    <head>
        <title>Custom directives</title>
        <style>
            [ng\:cloak], [ng-cloak], .ng-cloak{
                display: none;
            }
        </style>
    </head>
    <body>

        <div ng-controller="mainCtrl as o">
            <div  bb-player-list="bbPlayers" array-item="name | uppercase" ng-model="o"> 

            </div>
        </div>

        <script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.4.9/angular.min.js"></script>
        <script type="text/javascript" src="js/exam10.js"></script>
    </body>
</html>

var app10 = angular.module("app10", []);


app10.directive("bbPlayerList", DirectiveFunction);

function DirectiveFunction(){
    return function(scope, element, attrs){
            var data  = "";
            if(attrs["ngModel"] === "")
                data = scope[attrs["bbPlayerList"]];
            else
                data = scope[attrs["ngModel"]][attrs["bbPlayerList"]];

    }
}

app10.controller("mainCtrl", MainController);

function MainController(){
    this.bbPlayers = [
        {name: "Barry Bonds", avg: 0.298, hr: 762, obp: 0.444},
        {name: "Hank Aaron", avg: 0.305, hr: 755, obp: 0.374},
        {name: "Babe Ruth", avg: 0.342, hr: 714, obp: 0.474},
        {name: "Ted Williams", avg: 0.344, hr: 521, obp: 0.482}
    ];


}

In DirectiveFunction, there is a code smell in retrieving the data using ngModel,

    var data  = "";
    if(attrs["ngModel"] === "")
        data = scope[attrs["bbPlayerList"]];
    else
        data = scope[attrs["ngModel"]][attrs["bbPlayerList"]];

Reason to add this code smell is,

if controller is instantiated like mainCtrl as o then ngModel would be "o" otherwise ngModel will be "". This attribute ngModel will decide, how to access bbPlayers in DirectiveFunction?

Can I avoid this code smell in DirectiveFunction?

How does DirectiveFunction get immune to, the way mainCtrl is instantiated?

AL the X · Accepted Answer · 2016-11-18 01:51:59Z

You should consider require-ing the ngModelcontroller (and the MainController, if needed) inside your directive definition instead of inspecting the attrs inside of the link function:

function DirectiveFunction(){
  return {
    require: [ '^ngController', '?ngModel' ],
    link: (scope, element, attrs, controllers) => {
      const [ MainController, ngModel ] = controllers;

      data = main.bbPlayers;
    }
  };
}

It appears that you're trying to share data between controllers, however, which is best done through a shared, named service. Consider instead:

angular.module('app10')
  .factory('Players', function(){ // injectable, if you need it...
    return [
      {name: "Barry Bonds", avg: 0.298, hr: 762, obp: 0.444},
      {name: "Hank Aaron", avg: 0.305, hr: 755, obp: 0.374},
      {name: "Babe Ruth", avg: 0.342, hr: 714, obp: 0.474},
      {name: "Ted Williams", avg: 0.344, hr: 521, obp: 0.482}
    ];
  })
  .directive('bbPlayerList', function(){ // also injectable...
    return {
      controller: function(Players){ // injectable again...
        // Do stuff with `Players` if you need to...
      }
    }
  });

If you need to fetch the data from a server and cache the results, you can put that logic in the setup of Players or expose a method that will do the same:

angular.module('app10')
  .factory('Players', function(){ // injectable again...
    return {
      all: () => [
        {name: "Barry Bonds", avg: 0.298, hr: 762, obp: 0.444},
        {name: "Hank Aaron", avg: 0.305, hr: 755, obp: 0.374},
        {name: "Babe Ruth", avg: 0.342, hr: 714, obp: 0.474},
        {name: "Ted Williams", avg: 0.344, hr: 521, obp: 0.482}
      ]
    };
  });

Then call Players.all() inside any constructor to get all the players. You can easily replace the dummy data with a call to an endpoint with $http or $resource or Restangular.

Stack Exchange Network

Code smell in AngularJS custom directive

1 Answer 1

Hot Network Questions

Code smell in AngularJS custom directive

1 Answer 1

Related

Hot Network Questions