Applicability of Single Responsibility PrincipleDo you leverage the benefits of the open-closed principle?Single Responsibility Principle ImplementationSomething confusing about Single Responsibility PrincipleSingle Responsibility Principle: Responsibility unknownIdentifying Domain Services & Application Services when doing DDDApplication of Single Responsibility Principle on a ButtonWhen using the Single Responsibility Principle, what constitutes a “responsibility?”Single Responsibility Principle Violation?Does Template pattern violate Single Responsibility principle?Single responsibility principle - importer

Finding all intervals that match predicate in vector

Is it okay / does it make sense for another player to join a running game of Munchkin?

Is there a good way to store credentials outside of a password manager?

Modify casing of marked letters

Your magic is very sketchy

HashMap containsKey() returns false although hashCode() and equals() are true

How can a jailer prevent the Forge Cleric's Artisan's Blessing from being used?

Why is delta-v is the most useful quantity for planning space travel?

Why are on-board computers allowed to change controls without notifying the pilots?

What will be the benefits of Brexit?

Why Were Madagascar and New Zealand Discovered So Late?

Displaying the order of the columns of a table

What are the ramifications of creating a homebrew world without an Astral Plane?

Minimal reference content

Hide Select Output from T-SQL

What to do with wrong results in talks?

How to be diplomatic in refusing to write code that breaches the privacy of our users

There is only s̶i̶x̶t̶y one place he can be

Bash method for viewing beginning and end of file

Failed to fetch jessie backports repository

Curses work by shouting - How to avoid collateral damage?

Should my PhD thesis be submitted under my legal name?

What's a natural way to say that someone works somewhere (for a job)?

Trouble understanding overseas colleagues



Applicability of Single Responsibility Principle


Do you leverage the benefits of the open-closed principle?Single Responsibility Principle ImplementationSomething confusing about Single Responsibility PrincipleSingle Responsibility Principle: Responsibility unknownIdentifying Domain Services & Application Services when doing DDDApplication of Single Responsibility Principle on a ButtonWhen using the Single Responsibility Principle, what constitutes a “responsibility?”Single Responsibility Principle Violation?Does Template pattern violate Single Responsibility principle?Single responsibility principle - importer













7















I recently came by a seemingly trivial architectural problem. I had a simple repository in my code that was called like this (code is in C#):



var user = /* create user somehow */;
_userRepository.Add(user);
/* do some other stuff*/
_userRepository.SaveChanges();


SaveChanges was a simple wrapper that commits changes to database:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);



Then, after some time, I needed to implement new logic that would send email notifications every time a user was created in the system. Since there were many calls to _userRepository.Add() and SaveChanges around the system, I decided to update SaveChanges like this:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);
foreach (var newUser in dataContext.GetAddedUsers())

_eventService.RaiseEvent(new UserCreatedEvent(newUser ))




This way, external code could subscribe to UserCreatedEvent and handle the needed business logic that would send notifications.



But it was pointed out to me that my modification of SaveChanges violated the Single Responsibility principle, and that SaveChanges should just save and not fire any events.



Is this a valid point? It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function. And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.










share|improve this question






















  • Your retort is: "OK, so how would you write it so that it doesn't violate SRP but still allows a single point of modification?"

    – Robert Harvey
    4 hours ago






  • 2





    My observation is that raising an event does not add an additional responsibility. In fact, quite the opposite: it delegates the responsibility somewhere else.

    – Robert Harvey
    4 hours ago











  • I think your coworker is right, but your question is valid and useful, so upvoted!

    – Andres F.
    1 hour ago











  • There's no such thing as a definitive definition of a Single Responsibility. The person pointing out that it violates SRP is correct using their personal definition and you are correct using your definition. I think your design is perfectly fine with the caveat that this event isn't a one-off whereby other similar functionality is done in different ways. Consistency is far, far, far more important to pay attention to than some vague guideline like SRP which carried to the extreme ends up with tons of very easy to understand classes that nobody knows how to make work in a system.

    – Dunk
    1 min ago















7















I recently came by a seemingly trivial architectural problem. I had a simple repository in my code that was called like this (code is in C#):



var user = /* create user somehow */;
_userRepository.Add(user);
/* do some other stuff*/
_userRepository.SaveChanges();


SaveChanges was a simple wrapper that commits changes to database:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);



Then, after some time, I needed to implement new logic that would send email notifications every time a user was created in the system. Since there were many calls to _userRepository.Add() and SaveChanges around the system, I decided to update SaveChanges like this:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);
foreach (var newUser in dataContext.GetAddedUsers())

_eventService.RaiseEvent(new UserCreatedEvent(newUser ))




This way, external code could subscribe to UserCreatedEvent and handle the needed business logic that would send notifications.



But it was pointed out to me that my modification of SaveChanges violated the Single Responsibility principle, and that SaveChanges should just save and not fire any events.



Is this a valid point? It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function. And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.










share|improve this question






















  • Your retort is: "OK, so how would you write it so that it doesn't violate SRP but still allows a single point of modification?"

    – Robert Harvey
    4 hours ago






  • 2





    My observation is that raising an event does not add an additional responsibility. In fact, quite the opposite: it delegates the responsibility somewhere else.

    – Robert Harvey
    4 hours ago











  • I think your coworker is right, but your question is valid and useful, so upvoted!

    – Andres F.
    1 hour ago











  • There's no such thing as a definitive definition of a Single Responsibility. The person pointing out that it violates SRP is correct using their personal definition and you are correct using your definition. I think your design is perfectly fine with the caveat that this event isn't a one-off whereby other similar functionality is done in different ways. Consistency is far, far, far more important to pay attention to than some vague guideline like SRP which carried to the extreme ends up with tons of very easy to understand classes that nobody knows how to make work in a system.

    – Dunk
    1 min ago













7












7








7








I recently came by a seemingly trivial architectural problem. I had a simple repository in my code that was called like this (code is in C#):



var user = /* create user somehow */;
_userRepository.Add(user);
/* do some other stuff*/
_userRepository.SaveChanges();


SaveChanges was a simple wrapper that commits changes to database:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);



Then, after some time, I needed to implement new logic that would send email notifications every time a user was created in the system. Since there were many calls to _userRepository.Add() and SaveChanges around the system, I decided to update SaveChanges like this:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);
foreach (var newUser in dataContext.GetAddedUsers())

_eventService.RaiseEvent(new UserCreatedEvent(newUser ))




This way, external code could subscribe to UserCreatedEvent and handle the needed business logic that would send notifications.



But it was pointed out to me that my modification of SaveChanges violated the Single Responsibility principle, and that SaveChanges should just save and not fire any events.



Is this a valid point? It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function. And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.










share|improve this question














I recently came by a seemingly trivial architectural problem. I had a simple repository in my code that was called like this (code is in C#):



var user = /* create user somehow */;
_userRepository.Add(user);
/* do some other stuff*/
_userRepository.SaveChanges();


SaveChanges was a simple wrapper that commits changes to database:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);



Then, after some time, I needed to implement new logic that would send email notifications every time a user was created in the system. Since there were many calls to _userRepository.Add() and SaveChanges around the system, I decided to update SaveChanges like this:



void SaveChanges()

_dataContext.SaveChanges();
_logger.Log("User DB updated: " + someImportantInfo);
foreach (var newUser in dataContext.GetAddedUsers())

_eventService.RaiseEvent(new UserCreatedEvent(newUser ))




This way, external code could subscribe to UserCreatedEvent and handle the needed business logic that would send notifications.



But it was pointed out to me that my modification of SaveChanges violated the Single Responsibility principle, and that SaveChanges should just save and not fire any events.



Is this a valid point? It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function. And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.







architecture single-responsibility






share|improve this question













share|improve this question











share|improve this question




share|improve this question










asked 4 hours ago









Andre BorgesAndre Borges

6291811




6291811












  • Your retort is: "OK, so how would you write it so that it doesn't violate SRP but still allows a single point of modification?"

    – Robert Harvey
    4 hours ago






  • 2





    My observation is that raising an event does not add an additional responsibility. In fact, quite the opposite: it delegates the responsibility somewhere else.

    – Robert Harvey
    4 hours ago











  • I think your coworker is right, but your question is valid and useful, so upvoted!

    – Andres F.
    1 hour ago











  • There's no such thing as a definitive definition of a Single Responsibility. The person pointing out that it violates SRP is correct using their personal definition and you are correct using your definition. I think your design is perfectly fine with the caveat that this event isn't a one-off whereby other similar functionality is done in different ways. Consistency is far, far, far more important to pay attention to than some vague guideline like SRP which carried to the extreme ends up with tons of very easy to understand classes that nobody knows how to make work in a system.

    – Dunk
    1 min ago

















  • Your retort is: "OK, so how would you write it so that it doesn't violate SRP but still allows a single point of modification?"

    – Robert Harvey
    4 hours ago






  • 2





    My observation is that raising an event does not add an additional responsibility. In fact, quite the opposite: it delegates the responsibility somewhere else.

    – Robert Harvey
    4 hours ago











  • I think your coworker is right, but your question is valid and useful, so upvoted!

    – Andres F.
    1 hour ago











  • There's no such thing as a definitive definition of a Single Responsibility. The person pointing out that it violates SRP is correct using their personal definition and you are correct using your definition. I think your design is perfectly fine with the caveat that this event isn't a one-off whereby other similar functionality is done in different ways. Consistency is far, far, far more important to pay attention to than some vague guideline like SRP which carried to the extreme ends up with tons of very easy to understand classes that nobody knows how to make work in a system.

    – Dunk
    1 min ago
















Your retort is: "OK, so how would you write it so that it doesn't violate SRP but still allows a single point of modification?"

– Robert Harvey
4 hours ago





Your retort is: "OK, so how would you write it so that it doesn't violate SRP but still allows a single point of modification?"

– Robert Harvey
4 hours ago




2




2





My observation is that raising an event does not add an additional responsibility. In fact, quite the opposite: it delegates the responsibility somewhere else.

– Robert Harvey
4 hours ago





My observation is that raising an event does not add an additional responsibility. In fact, quite the opposite: it delegates the responsibility somewhere else.

– Robert Harvey
4 hours ago













I think your coworker is right, but your question is valid and useful, so upvoted!

– Andres F.
1 hour ago





I think your coworker is right, but your question is valid and useful, so upvoted!

– Andres F.
1 hour ago













There's no such thing as a definitive definition of a Single Responsibility. The person pointing out that it violates SRP is correct using their personal definition and you are correct using your definition. I think your design is perfectly fine with the caveat that this event isn't a one-off whereby other similar functionality is done in different ways. Consistency is far, far, far more important to pay attention to than some vague guideline like SRP which carried to the extreme ends up with tons of very easy to understand classes that nobody knows how to make work in a system.

– Dunk
1 min ago





There's no such thing as a definitive definition of a Single Responsibility. The person pointing out that it violates SRP is correct using their personal definition and you are correct using your definition. I think your design is perfectly fine with the caveat that this event isn't a one-off whereby other similar functionality is done in different ways. Consistency is far, far, far more important to pay attention to than some vague guideline like SRP which carried to the extreme ends up with tons of very easy to understand classes that nobody knows how to make work in a system.

– Dunk
1 min ago










4 Answers
4






active

oldest

votes


















3














Yes it is a violation of the single responsibility principle and a valid point.



A better design would be to have a separate process retrieve 'new users' from the repository and send the emails. Keeping track of which users have been sent an email, failures, resends etc etc.



This way you can handle errors, crashes and the like as well as avoiding your repository grabbing every requirement which has the idea that events happen "when something is committed to the database"



The repository doesn't know that a user you add is a new user. It's responsibility is simply storing the user.






share|improve this answer
































    1














    Be careful with premature event launching, because its side effects are hard (if possible) to undo.



    That said, consider the next premise. Creating users is one thing, it's persistence a different one.



    Creating users is a business-specific rule. A business concern. It might or might not involve persistence. It might involve more business operations, involving at the same time more database/network operations. Operations the persistence layer knows nothing about (and should not).



    It's not even true that _dataContext.SaveChanges(); has persisted the user successfully. It will depend on the db' transaction span. It could be true for databases like Mongodb, which transactions are atomic, but It could not for traditional RDBS implementing ACID transactions.



    There's a reason why transaction management happens at the business or service level. These are levels closer to the semantics of the business. They usually describe what user creation means, what to do when everything goes ok and what to do when not.




    It seems to me that the raising an event here is essentially the same
    thing as logging




    Note even close. Logging has no side effects. At least not the ones application events could have.




    it just says that such logic should be encapsulated in other classes,
    and it is OK for a repository to call these other classes




    Not true. SRP is not a class-specific concern. It also operates at higher-levels of abstractions, like layers, components, systems! It's about cohesion, keeping together what changes for the same reasons. If the user creation (use case) changes it's likely the moment and the reasons for the event to happen also changes.






    share|improve this answer

























    • +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

      – Andres F.
      1 hour ago






    • 1





      Exactly. Events denote certainity. Something happened but it's over.

      – Laiv
      1 hour ago












    • @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

      – Robert Harvey
      55 mins ago











    • Hmm. I have never seen this sort of events. Would you mind sharing some references?

      – Laiv
      42 mins ago


















    1















    Is this a valid point?




    Yes it is, although it depends a lot on the structure of your code. I don't have the full context so I will try to talk in general.




    It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function.




    It absolutely isn't. Logging is not part of the business flow, it can be disabled, it shouldn't cause (business) side effects and should not influence the state and heath of your application in any way, even if you were for some reason not able to log anything anymore. Now compare that with the logic you added.




    And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.




    SRP works in tandem with ISP (S and I in SOLID). You end up with many classes and methods that do very specific things and nothing else. They are very focused, very easy to update or replace, and in general easy(er) to test. Of course in practice you'll also have a few bigger classes that deal with the orchestration: they will have a number of dependencies, and they will focus not on atomised actions, but on business actions, which may require multiple steps. As long as the business context is clear, they can too be called single responsibility, but as you correctly said, as the code grows, you may want to abstract some of it into new classes / interfaces.



    Now back to your particular example. If you absolutely must send a notification whenever a user is created and maybe even perform other more specialised actions, then you could create a separate service that encapsulates this requirement, something like UserCreationService, which exposes one method, Add(user), which handles both the storage (the call to your repository) and the notification as a single business action. Or do it in your original snippet, after _userRepository.SaveChanges();






    share|improve this answer






























      1














      SRP is, theoretically, about people. The correct question is:



      Which "stakeholder" added the "send emails" requirement?



      If that stakeholder is also in charge of data persistence (unlikely but possible) then this does not violate SRP. Otherwise, it does.






      share|improve this answer
























        Your Answer








        StackExchange.ready(function()
        var channelOptions =
        tags: "".split(" "),
        id: "131"
        ;
        initTagRenderer("".split(" "), "".split(" "), channelOptions);

        StackExchange.using("externalEditor", function()
        // Have to fire editor after snippets, if snippets enabled
        if (StackExchange.settings.snippets.snippetsEnabled)
        StackExchange.using("snippets", function()
        createEditor();
        );

        else
        createEditor();

        );

        function createEditor()
        StackExchange.prepareEditor(
        heartbeatType: 'answer',
        autoActivateHeartbeat: false,
        convertImagesToLinks: false,
        noModals: true,
        showLowRepImageUploadWarning: true,
        reputationToPostImages: null,
        bindNavPrevention: true,
        postfix: "",
        imageUploader:
        brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
        contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
        allowUrls: true
        ,
        onDemand: true,
        discardSelector: ".discard-answer"
        ,immediatelyShowMarkdownHelp:true
        );



        );













        draft saved

        draft discarded


















        StackExchange.ready(
        function ()
        StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsoftwareengineering.stackexchange.com%2fquestions%2f389237%2fapplicability-of-single-responsibility-principle%23new-answer', 'question_page');

        );

        Post as a guest















        Required, but never shown

























        4 Answers
        4






        active

        oldest

        votes








        4 Answers
        4






        active

        oldest

        votes









        active

        oldest

        votes






        active

        oldest

        votes









        3














        Yes it is a violation of the single responsibility principle and a valid point.



        A better design would be to have a separate process retrieve 'new users' from the repository and send the emails. Keeping track of which users have been sent an email, failures, resends etc etc.



        This way you can handle errors, crashes and the like as well as avoiding your repository grabbing every requirement which has the idea that events happen "when something is committed to the database"



        The repository doesn't know that a user you add is a new user. It's responsibility is simply storing the user.






        share|improve this answer





























          3














          Yes it is a violation of the single responsibility principle and a valid point.



          A better design would be to have a separate process retrieve 'new users' from the repository and send the emails. Keeping track of which users have been sent an email, failures, resends etc etc.



          This way you can handle errors, crashes and the like as well as avoiding your repository grabbing every requirement which has the idea that events happen "when something is committed to the database"



          The repository doesn't know that a user you add is a new user. It's responsibility is simply storing the user.






          share|improve this answer



























            3












            3








            3







            Yes it is a violation of the single responsibility principle and a valid point.



            A better design would be to have a separate process retrieve 'new users' from the repository and send the emails. Keeping track of which users have been sent an email, failures, resends etc etc.



            This way you can handle errors, crashes and the like as well as avoiding your repository grabbing every requirement which has the idea that events happen "when something is committed to the database"



            The repository doesn't know that a user you add is a new user. It's responsibility is simply storing the user.






            share|improve this answer















            Yes it is a violation of the single responsibility principle and a valid point.



            A better design would be to have a separate process retrieve 'new users' from the repository and send the emails. Keeping track of which users have been sent an email, failures, resends etc etc.



            This way you can handle errors, crashes and the like as well as avoiding your repository grabbing every requirement which has the idea that events happen "when something is committed to the database"



            The repository doesn't know that a user you add is a new user. It's responsibility is simply storing the user.







            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited 3 hours ago

























            answered 4 hours ago









            EwanEwan

            42.2k33593




            42.2k33593























                1














                Be careful with premature event launching, because its side effects are hard (if possible) to undo.



                That said, consider the next premise. Creating users is one thing, it's persistence a different one.



                Creating users is a business-specific rule. A business concern. It might or might not involve persistence. It might involve more business operations, involving at the same time more database/network operations. Operations the persistence layer knows nothing about (and should not).



                It's not even true that _dataContext.SaveChanges(); has persisted the user successfully. It will depend on the db' transaction span. It could be true for databases like Mongodb, which transactions are atomic, but It could not for traditional RDBS implementing ACID transactions.



                There's a reason why transaction management happens at the business or service level. These are levels closer to the semantics of the business. They usually describe what user creation means, what to do when everything goes ok and what to do when not.




                It seems to me that the raising an event here is essentially the same
                thing as logging




                Note even close. Logging has no side effects. At least not the ones application events could have.




                it just says that such logic should be encapsulated in other classes,
                and it is OK for a repository to call these other classes




                Not true. SRP is not a class-specific concern. It also operates at higher-levels of abstractions, like layers, components, systems! It's about cohesion, keeping together what changes for the same reasons. If the user creation (use case) changes it's likely the moment and the reasons for the event to happen also changes.






                share|improve this answer

























                • +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

                  – Andres F.
                  1 hour ago






                • 1





                  Exactly. Events denote certainity. Something happened but it's over.

                  – Laiv
                  1 hour ago












                • @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

                  – Robert Harvey
                  55 mins ago











                • Hmm. I have never seen this sort of events. Would you mind sharing some references?

                  – Laiv
                  42 mins ago















                1














                Be careful with premature event launching, because its side effects are hard (if possible) to undo.



                That said, consider the next premise. Creating users is one thing, it's persistence a different one.



                Creating users is a business-specific rule. A business concern. It might or might not involve persistence. It might involve more business operations, involving at the same time more database/network operations. Operations the persistence layer knows nothing about (and should not).



                It's not even true that _dataContext.SaveChanges(); has persisted the user successfully. It will depend on the db' transaction span. It could be true for databases like Mongodb, which transactions are atomic, but It could not for traditional RDBS implementing ACID transactions.



                There's a reason why transaction management happens at the business or service level. These are levels closer to the semantics of the business. They usually describe what user creation means, what to do when everything goes ok and what to do when not.




                It seems to me that the raising an event here is essentially the same
                thing as logging




                Note even close. Logging has no side effects. At least not the ones application events could have.




                it just says that such logic should be encapsulated in other classes,
                and it is OK for a repository to call these other classes




                Not true. SRP is not a class-specific concern. It also operates at higher-levels of abstractions, like layers, components, systems! It's about cohesion, keeping together what changes for the same reasons. If the user creation (use case) changes it's likely the moment and the reasons for the event to happen also changes.






                share|improve this answer

























                • +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

                  – Andres F.
                  1 hour ago






                • 1





                  Exactly. Events denote certainity. Something happened but it's over.

                  – Laiv
                  1 hour ago












                • @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

                  – Robert Harvey
                  55 mins ago











                • Hmm. I have never seen this sort of events. Would you mind sharing some references?

                  – Laiv
                  42 mins ago













                1












                1








                1







                Be careful with premature event launching, because its side effects are hard (if possible) to undo.



                That said, consider the next premise. Creating users is one thing, it's persistence a different one.



                Creating users is a business-specific rule. A business concern. It might or might not involve persistence. It might involve more business operations, involving at the same time more database/network operations. Operations the persistence layer knows nothing about (and should not).



                It's not even true that _dataContext.SaveChanges(); has persisted the user successfully. It will depend on the db' transaction span. It could be true for databases like Mongodb, which transactions are atomic, but It could not for traditional RDBS implementing ACID transactions.



                There's a reason why transaction management happens at the business or service level. These are levels closer to the semantics of the business. They usually describe what user creation means, what to do when everything goes ok and what to do when not.




                It seems to me that the raising an event here is essentially the same
                thing as logging




                Note even close. Logging has no side effects. At least not the ones application events could have.




                it just says that such logic should be encapsulated in other classes,
                and it is OK for a repository to call these other classes




                Not true. SRP is not a class-specific concern. It also operates at higher-levels of abstractions, like layers, components, systems! It's about cohesion, keeping together what changes for the same reasons. If the user creation (use case) changes it's likely the moment and the reasons for the event to happen also changes.






                share|improve this answer















                Be careful with premature event launching, because its side effects are hard (if possible) to undo.



                That said, consider the next premise. Creating users is one thing, it's persistence a different one.



                Creating users is a business-specific rule. A business concern. It might or might not involve persistence. It might involve more business operations, involving at the same time more database/network operations. Operations the persistence layer knows nothing about (and should not).



                It's not even true that _dataContext.SaveChanges(); has persisted the user successfully. It will depend on the db' transaction span. It could be true for databases like Mongodb, which transactions are atomic, but It could not for traditional RDBS implementing ACID transactions.



                There's a reason why transaction management happens at the business or service level. These are levels closer to the semantics of the business. They usually describe what user creation means, what to do when everything goes ok and what to do when not.




                It seems to me that the raising an event here is essentially the same
                thing as logging




                Note even close. Logging has no side effects. At least not the ones application events could have.




                it just says that such logic should be encapsulated in other classes,
                and it is OK for a repository to call these other classes




                Not true. SRP is not a class-specific concern. It also operates at higher-levels of abstractions, like layers, components, systems! It's about cohesion, keeping together what changes for the same reasons. If the user creation (use case) changes it's likely the moment and the reasons for the event to happen also changes.







                share|improve this answer














                share|improve this answer



                share|improve this answer








                edited 1 hour ago

























                answered 2 hours ago









                LaivLaiv

                6,89311241




                6,89311241












                • +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

                  – Andres F.
                  1 hour ago






                • 1





                  Exactly. Events denote certainity. Something happened but it's over.

                  – Laiv
                  1 hour ago












                • @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

                  – Robert Harvey
                  55 mins ago











                • Hmm. I have never seen this sort of events. Would you mind sharing some references?

                  – Laiv
                  42 mins ago

















                • +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

                  – Andres F.
                  1 hour ago






                • 1





                  Exactly. Events denote certainity. Something happened but it's over.

                  – Laiv
                  1 hour ago












                • @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

                  – Robert Harvey
                  55 mins ago











                • Hmm. I have never seen this sort of events. Would you mind sharing some references?

                  – Laiv
                  42 mins ago
















                +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

                – Andres F.
                1 hour ago





                +1 Very good point about the transaction span. It can be premature to assert the user has been created, because rollbacks can happen; and unlike with a log, it's likely some other part of the app does something with the event.

                – Andres F.
                1 hour ago




                1




                1





                Exactly. Events denote certainity. Something happened but it's over.

                – Laiv
                1 hour ago






                Exactly. Events denote certainity. Something happened but it's over.

                – Laiv
                1 hour ago














                @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

                – Robert Harvey
                55 mins ago





                @Laiv: Except when they don't. Microsoft has all sorts of events prefixed with Before or Preview that make no guarantees at all about certainty.

                – Robert Harvey
                55 mins ago













                Hmm. I have never seen this sort of events. Would you mind sharing some references?

                – Laiv
                42 mins ago





                Hmm. I have never seen this sort of events. Would you mind sharing some references?

                – Laiv
                42 mins ago











                1















                Is this a valid point?




                Yes it is, although it depends a lot on the structure of your code. I don't have the full context so I will try to talk in general.




                It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function.




                It absolutely isn't. Logging is not part of the business flow, it can be disabled, it shouldn't cause (business) side effects and should not influence the state and heath of your application in any way, even if you were for some reason not able to log anything anymore. Now compare that with the logic you added.




                And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.




                SRP works in tandem with ISP (S and I in SOLID). You end up with many classes and methods that do very specific things and nothing else. They are very focused, very easy to update or replace, and in general easy(er) to test. Of course in practice you'll also have a few bigger classes that deal with the orchestration: they will have a number of dependencies, and they will focus not on atomised actions, but on business actions, which may require multiple steps. As long as the business context is clear, they can too be called single responsibility, but as you correctly said, as the code grows, you may want to abstract some of it into new classes / interfaces.



                Now back to your particular example. If you absolutely must send a notification whenever a user is created and maybe even perform other more specialised actions, then you could create a separate service that encapsulates this requirement, something like UserCreationService, which exposes one method, Add(user), which handles both the storage (the call to your repository) and the notification as a single business action. Or do it in your original snippet, after _userRepository.SaveChanges();






                share|improve this answer



























                  1















                  Is this a valid point?




                  Yes it is, although it depends a lot on the structure of your code. I don't have the full context so I will try to talk in general.




                  It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function.




                  It absolutely isn't. Logging is not part of the business flow, it can be disabled, it shouldn't cause (business) side effects and should not influence the state and heath of your application in any way, even if you were for some reason not able to log anything anymore. Now compare that with the logic you added.




                  And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.




                  SRP works in tandem with ISP (S and I in SOLID). You end up with many classes and methods that do very specific things and nothing else. They are very focused, very easy to update or replace, and in general easy(er) to test. Of course in practice you'll also have a few bigger classes that deal with the orchestration: they will have a number of dependencies, and they will focus not on atomised actions, but on business actions, which may require multiple steps. As long as the business context is clear, they can too be called single responsibility, but as you correctly said, as the code grows, you may want to abstract some of it into new classes / interfaces.



                  Now back to your particular example. If you absolutely must send a notification whenever a user is created and maybe even perform other more specialised actions, then you could create a separate service that encapsulates this requirement, something like UserCreationService, which exposes one method, Add(user), which handles both the storage (the call to your repository) and the notification as a single business action. Or do it in your original snippet, after _userRepository.SaveChanges();






                  share|improve this answer

























                    1












                    1








                    1








                    Is this a valid point?




                    Yes it is, although it depends a lot on the structure of your code. I don't have the full context so I will try to talk in general.




                    It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function.




                    It absolutely isn't. Logging is not part of the business flow, it can be disabled, it shouldn't cause (business) side effects and should not influence the state and heath of your application in any way, even if you were for some reason not able to log anything anymore. Now compare that with the logic you added.




                    And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.




                    SRP works in tandem with ISP (S and I in SOLID). You end up with many classes and methods that do very specific things and nothing else. They are very focused, very easy to update or replace, and in general easy(er) to test. Of course in practice you'll also have a few bigger classes that deal with the orchestration: they will have a number of dependencies, and they will focus not on atomised actions, but on business actions, which may require multiple steps. As long as the business context is clear, they can too be called single responsibility, but as you correctly said, as the code grows, you may want to abstract some of it into new classes / interfaces.



                    Now back to your particular example. If you absolutely must send a notification whenever a user is created and maybe even perform other more specialised actions, then you could create a separate service that encapsulates this requirement, something like UserCreationService, which exposes one method, Add(user), which handles both the storage (the call to your repository) and the notification as a single business action. Or do it in your original snippet, after _userRepository.SaveChanges();






                    share|improve this answer














                    Is this a valid point?




                    Yes it is, although it depends a lot on the structure of your code. I don't have the full context so I will try to talk in general.




                    It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function.




                    It absolutely isn't. Logging is not part of the business flow, it can be disabled, it shouldn't cause (business) side effects and should not influence the state and heath of your application in any way, even if you were for some reason not able to log anything anymore. Now compare that with the logic you added.




                    And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.




                    SRP works in tandem with ISP (S and I in SOLID). You end up with many classes and methods that do very specific things and nothing else. They are very focused, very easy to update or replace, and in general easy(er) to test. Of course in practice you'll also have a few bigger classes that deal with the orchestration: they will have a number of dependencies, and they will focus not on atomised actions, but on business actions, which may require multiple steps. As long as the business context is clear, they can too be called single responsibility, but as you correctly said, as the code grows, you may want to abstract some of it into new classes / interfaces.



                    Now back to your particular example. If you absolutely must send a notification whenever a user is created and maybe even perform other more specialised actions, then you could create a separate service that encapsulates this requirement, something like UserCreationService, which exposes one method, Add(user), which handles both the storage (the call to your repository) and the notification as a single business action. Or do it in your original snippet, after _userRepository.SaveChanges();







                    share|improve this answer












                    share|improve this answer



                    share|improve this answer










                    answered 43 mins ago









                    asyncasync

                    51459




                    51459





















                        1














                        SRP is, theoretically, about people. The correct question is:



                        Which "stakeholder" added the "send emails" requirement?



                        If that stakeholder is also in charge of data persistence (unlikely but possible) then this does not violate SRP. Otherwise, it does.






                        share|improve this answer





























                          1














                          SRP is, theoretically, about people. The correct question is:



                          Which "stakeholder" added the "send emails" requirement?



                          If that stakeholder is also in charge of data persistence (unlikely but possible) then this does not violate SRP. Otherwise, it does.






                          share|improve this answer



























                            1












                            1








                            1







                            SRP is, theoretically, about people. The correct question is:



                            Which "stakeholder" added the "send emails" requirement?



                            If that stakeholder is also in charge of data persistence (unlikely but possible) then this does not violate SRP. Otherwise, it does.






                            share|improve this answer















                            SRP is, theoretically, about people. The correct question is:



                            Which "stakeholder" added the "send emails" requirement?



                            If that stakeholder is also in charge of data persistence (unlikely but possible) then this does not violate SRP. Otherwise, it does.







                            share|improve this answer














                            share|improve this answer



                            share|improve this answer








                            edited 32 mins ago

























                            answered 38 mins ago









                            user949300user949300

                            5,84511528




                            5,84511528



























                                draft saved

                                draft discarded
















































                                Thanks for contributing an answer to Software Engineering Stack Exchange!


                                • Please be sure to answer the question. Provide details and share your research!

                                But avoid


                                • Asking for help, clarification, or responding to other answers.

                                • Making statements based on opinion; back them up with references or personal experience.

                                To learn more, see our tips on writing great answers.




                                draft saved


                                draft discarded














                                StackExchange.ready(
                                function ()
                                StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fsoftwareengineering.stackexchange.com%2fquestions%2f389237%2fapplicability-of-single-responsibility-principle%23new-answer', 'question_page');

                                );

                                Post as a guest















                                Required, but never shown





















































                                Required, but never shown














                                Required, but never shown












                                Required, but never shown







                                Required, but never shown

































                                Required, but never shown














                                Required, but never shown












                                Required, but never shown







                                Required, but never shown







                                Popular posts from this blog

                                名間水力發電廠 目录 沿革 設施 鄰近設施 註釋 外部連結 导航菜单23°50′10″N 120°42′41″E / 23.83611°N 120.71139°E / 23.83611; 120.7113923°50′10″N 120°42′41″E / 23.83611°N 120.71139°E / 23.83611; 120.71139計畫概要原始内容臺灣第一座BOT 模式開發的水力發電廠-名間水力電廠名間水力發電廠 水利署首件BOT案原始内容《小檔案》名間電廠 首座BOT水力發電廠原始内容名間電廠BOT - 經濟部水利署中區水資源局

                                Prove that NP is closed under karp reduction?Space(n) not closed under Karp reductions - what about NTime(n)?Class P is closed under rotation?Prove or disprove that $NL$ is closed under polynomial many-one reductions$mathbfNC_2$ is closed under log-space reductionOn Karp reductionwhen can I know if a class (complexity) is closed under reduction (cook/karp)Check if class $PSPACE$ is closed under polyonomially space reductionIs NPSPACE also closed under polynomial-time reduction and under log-space reduction?Prove PSPACE is closed under complement?Prove PSPACE is closed under union?

                                Is my guitar’s action too high? Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 23, 2019 at 23:30 UTC (7:30pm US/Eastern)Strings too stiff on a recently purchased acoustic guitar | Cort AD880CEIs the action of my guitar really high?Μy little finger is too weak to play guitarWith guitar, how long should I give my fingers to strengthen / callous?When playing a fret the guitar sounds mutedPlaying (Barre) chords up the guitar neckI think my guitar strings are wound too tight and I can't play barre chordsF barre chord on an SG guitarHow to find to the right strings of a barre chord by feel?High action on higher fret on my steel acoustic guitar