SOLID Principles – Does a Method Handling Both GET and POST Requests Violate SRP?

single-responsibilitysolid

I'm starting with Zend Framework 3 and I saw an example on the tutorial that goes like this:

public function addAction()
{
    $form = new AlbumForm();
    $form->get('submit')->setValue('Add');

    $request = $this->getRequest();

    if (! $request->isPost()) {       // <-------------- HERE!!!!
        return ['form' => $form];
    }

    $album = new Album();
    $form->setInputFilter($album->getInputFilter());
    $form->setData($request->getPost());

    if (! $form->isValid()) {
        return ['form' => $form];
    }

    $album->exchangeArray($form->getData());
    $this->table->saveAlbum($album);
    return $this->redirect()->toRoute('album');
}

Now, considering what Robert C. Martin says about SRP (from wikipedia):

A class or module should have one, and only one, reason to change.

From my perspective, given the specifications for the creation form changes (maybe we now have to load data in a dropdown box) this would affect the GET request. However, given the specification for a redirection changes (we now redirect to /index instead of /details) this would affect the POST request.

Since both are in the same method, does this violates SRP, or am I missing something?

Disclaimer: I come from Laravel, where we have 1 method for each request; so having a check like if($request->isPost) {} feels wrong. But this is official Zend Framework 3 documentation, so maybe I'm missing something.

Best Answer

I may be misunderstanding, but it seems there are two questions here: one about HTTP endpoint design, and another about function design.

Endpoints

I am not familiar with PHP, nor the Zend Framework. But in designing a REST(ish) API, it is quite common to have an endpoint api.mysite/users representing a collection (in this case of users). A GET to said endpoint would return a collection of all the users, and a POST to the endpoint would create a new user.

I cannot read your code, so I can't tell whether this is the case for you. But in general, different verbs on a single endpoint should be related.

Functions

I come from Laravel, where we have 1 method for each request; so having a check like if($request->isPost) {} feels wrong.

You're correct. In fact, a boolean-valued function isPost is wrong. The correct approach would be to have a method getRequestMethod that returns an enumeration - or in languages which lack such support, a string (shudders). The case statement would defer to the appropriate method. Of course, this logic could be taken care of behind the scenes, and then you would just fill in the appropriate get, post, functions etc (I use a framework that takes this approach).

Related Topic