1

I was reading http://misko.hevery.com/attachments/Guide-Writing%20Testable%20Code.pdf (see, especially, page8) and watching Misko's Youtube videos on writing testable code, and it occurs to me that the way Angular does DI forces you to break the law of Demeter.

Simplifying from the PDF, an example of a Java constructor breaking the law of Demeter:

class AccountView {
  boolean isAdmin;
  AccountView(AccountService) {
    isAdmin = AccountService.getCurrentUser().getProfile().isAdmin();
  }
}

because the class only needs whether the user is an admin, and not the AccountService.

It seems like Angular forces you to break the law of Demeter with it's DI. I can't see an alternative to the following:

.controller('AccountServiceController', 
  ['AccountService', 
  function(AccountService) {
    this.user = AccountService.getCurrentUser().getProfile().isAdmin();
  }]
);

We could inject a user if this were a controller that was controller a router with resolve parameters, but not for the general case. Any thoughts? Note that I'm assuming that only AccountService is a singleton, and each subsequent object is an instance (and cannot be DI'd).

Robert Balicki
  • 1,583
  • 2
  • 16
  • 24

1 Answers1

1

I'm not sure the Angular example you provided or Angular DI in general is breaking the law of demeter. Angular dependency injection allows you to write very testable code.

Let's assume the real AccountService.getCurrentUser(); is a very expensive network operation. We definitely do not want to call this method in a test. With Angular's dependency injection we can mock AccountService so we don't have to call the real one.

Let's write a test for AccountServiceController:

controller

.controller('AccountServiceController', 
  ['$scope', 'AccountService', 
  function($scope, AccountService) {
    $scope.user = AccountService.getCurrentUser();
  }]
);

test

describe('AccountServiceController function', function() {

  describe('AccountServiceController', function() {
    var $scope;

    beforeEach(module('myApp'));

    beforeEach(inject(function($rootScope, $controller) {
      $scope = $rootScope.$new();
      $controller('AccountServiceController', {
        $scope: $scope
        AccountService: {
          getCurrentUser: function () {
            return {
              name: 'Austin Pray',
              id: 1111
            };
          }
        }
      });
    }));

    it('should get the current user', function() {
      expect(typeof $scope.user.name).toBe('string');
    });
  });
});

We have avoided the expensive network operation and the controller is not coupled in any way to the AccountService's internals.

Austin Pray
  • 4,926
  • 2
  • 22
  • 30
  • I don't think it actually results in hard to test code, but I'd point out (from page 21 of the linked pdf) that: "Law of Demeter violations occur when one method acts as a locator in addition to its primary responsibility ... You should never have to mock out a setter or getter." Perhaps a better example would be AccountService.getCurrentUser().getProfile().isAdmin(). From pg 17: "A "Law of Demeter" violation ... Symptom: seeing more than one period “.” in a method chaining where the methods are getters". My example doesn't have this, but Angular DI forces it in other cases. – Robert Balicki Dec 18 '14 at 17:20
  • Couldn't you just make services like `currentUserService` until you only have one dot? – Austin Pray Dec 18 '14 at 17:30
  • You'd need to make a `currentProfileService` to get to one dot, but that smells as well, right? Actually, even `currentProfileService` would require two dots (`currentProfileService.getCurrentProfile().isAdmin()`). Regardless, it seems like now we're dealing with an unnecessary proliferation of classes. – Robert Balicki Dec 18 '14 at 17:39