Am I violating the SRP in this code? [closed]

Question

Here's a sample code that I made up :

<?php

class RegisterController {
    public $userRepo;
    public $blogRepo;
    public $mailer;
    public $avatarRepo;

    public function __construct(UserRepository $userRepo, AvatarRepository $avatarRepo, BlogRepository $blogRepo, Mailer $mailer) {
        $this->userRepo = $userRepo;
        $this->blogRepo = $blogRepo;
        $this->mailer = $mailer;
        $this->avatarRepo = $avatarRepo;
    }

    public function register(Request $request) {
        $user = $this->userRepo->register($request);
        $this->avatarRepo->uploadFor($user, $request->input('base64_avatar'));
        $this->blogRepo->makeNewFor($user);
        $this->mailer->sendRegisteredConfirmationTo($user);
    }
}

It feels like my controller is doing too many things :

• Creates a user
• Upload the avatar and link it to the user
• Create a blog and link it to the user
• Send a mail to the user

But it also feels like my controller is only doing one thing here : "Registering a user".

In the same manner, what if the RegisterController now also has a unregister() method ? Is it going to violate the SRP ? Since one could say it'll then handle registering AND unregistering, but one could also say it'll still only handle the concept of registering.

Where is the line that says "the scope of this class or this method is too wide, there should be two classes/methods" ?

Answer 1

Am I violating the SRP in this code?

Not really. And if you are, it doesn't matter.

When a user registers, you want to perform 4 actions, which means that these 4 actions need to be applied in one place, and a register function is the right place for it.

Note that the original definition of "responsibility" in the context of SRP is "a reason for change". So you could look at it like this: The only responsibility of the register function is to register a user. You don't call it when a user unregisters, when a user changes their password, when a user needs a password reset, etc.

You also have separated the actual implementations in the appropriate classes, so I don't see a problem here.

Misc

• does the register method really need the whole $request class? It should be the task of the controller to parse the user input, and only pass on needed stuff (eg password, username, etc).
• is there a reason that your fields are public?