0
\$\begingroup\$

We have a base module consuming the company API. In each app I've written almost always a service per controller. Because most controllers are specific to a entity (a single user, a single post, list of posts written by a user), when I initialize them to query the data I have to request for that Id. So the service requires the controller to specify an Id.

Because of this I've decided the following: this services return a function which will construct the service object receiving the Id as parameter. Then the controller builds the service. With this I'm reusing those services, i.e.: A user edit his personal information on AccountController and administrator edit the personal user information on UsersListController both building the UserEditService passing the Id.

    // View User Service
function(UserResource, $log, ErrorHandler) {
    var fn = function(userId) {
       var id = userId,
           svc = {
               user: {}
           };

       svc.get = function(){
           UserResource.get(userId)
               .then(function(res)  {
                    svc.user = res.data.user;
                    svc.interests = res.data.interests;
               }, ErrorHandler.error);
       };
       return svc;
    };
    return fn;
}

// View User Controller
function(UserViewService, $scope, $stateParams) {
    var svc = new UserViewService(stateParams.id);

    this.user = svc.user;
    function watchUser() {
        return svc.user;
    };
    $scope.$watch(watchUser, function(current, previous) {
        svc.user = current;
    });
};

Am I implementing some ugly/confuse code to deal with this? I'm only doing this on applications, the base modules services and methods are encapsulated, they always receive the Id in methods parameters. But on apps, each service would had to receive the Id on each parameter in order to be reused across several controllers (they could be loaded in the same session consuming the same service).

I've tried to avoid this extra service and calling the UserResource on controller but I always end up with controller handling business operations.

\$\endgroup\$
1
\$\begingroup\$

"A service per controller" sounds like a bad architecture. A good service should 'know' nothing about controllers and presentation details. Controllers should be thin, and any repeating code there should be abstracted into general purpose services not specific to controllers.

"UserViewService" should never be a name of Controller! And comments are not there to fix the confusion created by bad naming! In Angular, there are Services, Controllers, and Views - these are your Angular templates. You can have UserService and UserController, then your template can be called user-view.

Having new inside your Controller looks like a "code smell" to me. Controllers are only executed once upon initialization, and their purpose is to glue your data to your view (via Angular's $scope (which is in the ViewModel layer). The data should be either ready or you should get it from a service. Any other work like object creation is bloating your controller with too many responsibilities.

Further, it is Angular's job to create Services for you, so no need for new UserViewService whatsoever.

This function naming is misleading:

function watchUser() {
    return svc.user;
};

The name should tell roughly what function does, so I can skip this function's definition and still understand the rest of the code. This is a standard "getter"-function and should be name like that:

function getUser (svc) {
    return svc.user;
}

Note. Your code looks cleaner without ; after function declarations.

You watch svc.user and then update it upon change - this looks like a loop and may lead to your browser crash. Read best practices on using Angulars watch.

\$\endgroup\$

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.