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

Single Responsibility of Services #456

Closed
robrothedev opened this issue Jul 28, 2015 · 6 comments
Closed

Single Responsibility of Services #456

robrothedev opened this issue Jul 28, 2015 · 6 comments
Labels

Comments

@robrothedev
Copy link

Very good information in this style guide. I did have a question on services and single responsibility and how granular this needs to be. For instance, I have video app with crud capabilities. Initially my service structure is as follows:

// videos.service.js
(function() {

    'use strict';

    function Videos($http) {
        var service = {
            getVideos: getVideos,
            getVideo: getVideo,
            saveVideo: saveVideo,
            deleteVideo: deleteVideo
        }

        return service;

        function getVideos() {
            // return video list
        }

        function getVideo() {
            // return video by id
        }

        function saveVideo() {
            // add/edit video
        }

        function deleteVideo() {
            // delete video
        }
    }

})();

Would it be better to separate out the functionality like this?

// videos.service.js
(function() {

    'use strict';

    function Videos($http) {
        var service = {
            getVideos: getVideos,
            getVideo: getVideo
        }

        return service;

        function getVideos() {
            // return video list
        }

        function getVideo() {
            // return video by id
        }
    }

})();

// videos_crud.service.js
(function() {

    'use strict';

    function VideosCrud($http) {
        var service = {
            saveVideo: saveVideo,
            deleteVideo: deleteVideo
        }

        return service;

        function saveVideo() {
            // add/edit video
        }

        function deleteVideo() {
            // delete video
        }
    }

})();

This might come down to personal preference but thought I would get any insight someone might have regarding this.

Thanks for any suggestions.

@viniciuskneves
Copy link

I'm not an expert in Angular but as a CRUD service it might include all CRUD actions so you shouldn't break it in videos (with gets) and video_crud (with save/create and delete). Otherwise the meaning of CRUD service might get confused.

Nevertheless, wait for more opinions about it :)

@tedvanderveen
Copy link

At first sight your initial service deals with CRUD against probably the same REST API endpoint, handling your Video related persistence functionality. It has one and the same depenency on $http. So I would see no reason to split things up, in that case you may even end up with these split services depending on each other in case you need to implement an udateVideo method that needs need to call getVideo to load some metadat, for example.

@robrothedev
Copy link
Author

@viniciuskneves That idea would provide practicality when providing an app for public vs. admin users at which point the api server should be discerning between the two.

@tedvanderveen I tend to agree with that also. I guess the separation granularity probably exists more for the type of module itself rather than what the module is doing.

@johnpapa
Copy link
Owner

The first question is 'why?'. Then .... 'what benefit will you get from this?`

I don't think there is ay obvious benefit. Unless you can define that clearly and it is not filled with IFs and MAYBEs, then it may have value.

@johnpapa
Copy link
Owner

johnpapa commented Aug 5, 2015

Closing the issue as I dont think this will be added to the guide, but the question is good and I beleive answered.

My perspective: there's not enough value in doing that

@johnpapa johnpapa closed this as completed Aug 5, 2015
@robrothedev
Copy link
Author

@johnpapa Thanks for the input. Got the clarification I needed.

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

No branches or pull requests

4 participants