code adherence to single responsibility principle & unit testing

Question

I have lately been reading on the Single Responsibility Principle concept, and in theory I agree a lot with it. I am having difficulty coming to terms on which code can be exactly classified as violating the principle. I would like to implement this principle, so as to gear towards Test-Driven Development.

Take for example the code below:

public void MarkAsSuccessful(PaymentMethodSpecific paymentMethod, bool requiresManualIntervention)
    {
            this.Paid = true;
            this.PaidOn = CS.General_v3.Util.Date.Now;
            this.PaidByPaymentMethod = paymentMethod;
            this.RequiresManualIntervention = requiresManualIntervention;

            this.Update();

        this.CreateAndSendNotificationRegardingImmediatePayment();

        this.SendPaymentSuccessfulEmails();

    }

This is placed in a class called PaymentRequest, which basically is a class which handles logic related to payment in an e-commerce application. The method above marks the request as 'successful'. This must mark the columns paid, as well as other information, as well as send a notification that this was successful, and also send emails.

For example, when it comes to unit testing - It is very difficult to unit-test this method as I have no way to know that the notification was actually created and sent, as well as the payment emails have been sent. Would like to know how more experienced people on the SRP concept would approach such an example.

You may have deeper problems here. Do you see why a method named `CreateAndSend...` would violate SRP? — Austin Salonen, Aug 23 '12 at 16:29
@AustinSalonen I see what you mean, and I disagree: what if he always creates **and** sends notifications? If that method just calls 2 other methods, it's fine. A very common mistake with SRP is trying to apply it 100% of the time. — Louis Kottmann, Aug 23 '12 at 16:48

score 1 · Accepted Answer · answered Aug 23 '12 at 16:42

My guess is that you have a PaymentProcessor class merged with/embedded in your PaymentRequest class.

PaymentProcessor is basically a finite-state machine that a PaymentRequest travels through.

When constructing this PaymentProcessor you would inject objects that would handle the "immediate payment notifications" and emails in general. This way you test all that states independently and the processor by injecting mock objects that assert that expected conditions occurred.

Single responsibility by itself is a great concept but the power really comes when you start investing in SOLID as a whole.

@AustinSalonen Yes, I agree that definitely this `PaymentRequest` class is dealing with more responsibilities than it actually should. Injecting/mocking objects would be a good way to go, to test that method, without actually having to verify that the actual notification has been sent. — Karl Cassar, Aug 24 '12 at 09:05

code adherence to single responsibility principle & unit testing

1 Answers1