Confusion about single responsibility in OOP

Question

Let's consider the following example:

class User
{

}

class FirstUseNotification
{
    function show(User user)
    {
        // check if it was already shown, return if so
        // show a notification
        // mark it as shown in the db or whatever
    }
}

class SomeController
{
    function someMethod()
    {
        firstUseNotification->show(user);
    }
}

The show() method seems to break single responsibility by doing 3 things. So i figure this could be rewritten as such:

class User
{

}

class FirstUseNotification
{
    function show(User user)
    {
        // show a notification
    }

    function shouldShow(User user)
    {
        // return true if not yet shown
    }

    function markAsShown(User user)
    {
        // flag notification as shown
    }
}

class SomeController
{
    function someMethod()
    {
        if (firstUseNotification->shouldShow(user)) 
        {
            firstUseNotification->show(user);
            firstUseNotification->markAsShown(user);
        }
    }
}

So here's what i'm interested in:

1. Am i correct to assume that in second example the notification class is now OK with single responsibility principle?
2. All of the things that were happening in show() method are gone, but ... they are simply relocated to a method in a controller, so shouldn't it mean that this controller method now breaks single responsibility? If so, how can this be done to comply?

Answer 1

The single responsibility principle (SRP) is often stated in the form of a quote by Robert C. Martin:

A class should have only one reason to change.

In this case, the purpose of your FirstUseNotification class is to show a notification to a first-time user. So the only reason this class should need to change is if this purpose changes; and that is one reason, so the SRP is satisfied.

Note that this definition applies to a class, not to a method. That said, splitting this method into three methods probably violates the SRP, because if a user of this class needs to call three methods instead of one, then that user class has the responsibility of checking whether to show the notification, and marking the user as shown, in addition to the user class's own responsibility. FirstUseNotification's responsibility is to "show a notification to a first-time user", not to provide an API that allows other classes to do that when it is not their responsibility.

In practice the FirstUserNotification class might have other reasons to change, if the details of how it shows the notification or accesses the database change. This can ideally be prevented by having stable interfaces for the notification and database classes, so that changes to those classes don't require changes to FirstUseNotification or other classes which show notifications and/or access the database. In practice this is not always 100% achieved, in which case the FirstUseNotification class might have some responsibilities to do with the details of showing a notification or accessing the database. But in theory, if those other classes handle their own responsibilities properly, then this class has only one reason to change.