17

The Single Responsibility Principle states that "a class should have one reason the change".

In the MVC pattern, the Controller's job is to mediate between the View and the Model. It offers an interface for the View to report actions made by the user on the GUI (e.g. allowing the View to call controller.specificButtonPressed()), and is able to call the appropriate methods on the Model in order to manipulate it's data or invoke it's operations (e.g. model.doSomething()).

This means that:

  • The Controller needs to know about the GUI, in order to offer the View a suitable interface to report user actions.
  • It also needs to know about the logic in the Model, in order to be able to invoke the appropriate methods on the Model.

That means that is has two reasons to change: a change in the GUI, and a change in the buisness logic.

If the GUI changes, e.g. a new button is added, the Controller might need to add a new method to allow the View to report a user press on this button.

And if the business logic in the Model changes, the Controller might have to change in order to invoke the correct methods on the Model.

Therefore, the Controller has two possible reasons to change. Does it break SRP?

  • 2
    A controller is a 2-way street, it's not your typical top-down or bottom-up approach. There is no possibility for it to abstract one of its dependencies because the controller is the abstraction itself. Due to the nature of the pattern it is not possible to adhere to the SRP here. In short: yes, it violates SRP but that is unavoidable. – Jeroen Vannevel May 3 '14 at 23:18
  • 1
    What is the point of the question? If we all answer "yes, it does", what then? What if the answer is "no"? What is the real problem you are trying to solve with this question? – Bryan Oakley May 4 '14 at 0:52
  • 1
    "a reason to change" does not mean "code that changes". If you make seven typos in your variable names for the class, does that class now have 7 responsibilities? No. If you have more than one variable or more than one function, you also might still have only a single responsibility. – Bob May 4 '14 at 8:15
14

If you continue consequently to reason about the SRP you will come to notice that "single responsiblity" is actually a spongy term. Our human brain is somehow able to distinguish between different responsibilities and multiple responsibilities can be abstracted into one "general" responsibility. For example, imagine in a common 4 person family there is one familiy member responsible for making the breakfast. Now, to do this, one has to boil eggs and toast bread and of course set up some healthy cup of green tea (yes, green tea is best). This way you can break "making breakfast" down into smaller pieces which are together abstracted to "making breakfast". Notice that each piece is also a responsibility that could e.g. be delegated to another person.

Back to the MVC: if mediating between model and view ist not one responsibility but two, then what would be the next abstraction layer above, combining those two? If you can't find one you either didn't abstract it correctly or there is none which means you got it all right. And i feel that is the case with a controller, handling a view and a model.

  • 1
    Just as an added note. If you have controller.makeBreakfast() and controller.wakeUpFamily() then that controller would be breaking SRP, but not because it's a controller, just because it has more than one responsibility. – Bob May 4 '14 at 8:18
  • Thanks for answering, not sure I'm following you. Do you agree that the controller has more than one responsibility? The reason I think so is because it has two reasons to change (I think): a change in the model and a change in the view. Do you agree with this? – Aviv Cohn May 4 '14 at 21:59
  • 1
    Yes, i can agree that the controller has more than one responsibility. However these are "lower" responsibilities and that is not a problem because at the highest abstraction level it only has one responsibility (combining the lower ones you mention) and so it does not violate the SRP. – valenterry May 5 '14 at 12:55
  • 1
    Finding the right level of abstraction is definitely important. The "make breakfast" example is a good one -- to complete a single responsibility there are often a number of tasks that must be accomplished. As long as the controller is only orchestrating these tasks, it follows SRP. But if it knows too much about boiling eggs, making toast, or brewing tea, then it would violate SRP. – Allan May 6 '14 at 4:47
  • This answer makes sense to me. Thank You Valenterry. – J86 Feb 12 '15 at 9:33
9

If a class has "two possible reasons to change", then yes, it violates SRP.

A controller should usually be lightweight and have the single responsibility of manipulating the domain/model in response to some GUI-driven event. We can consider each of these manipulations to basically be use cases or features.

If a new button is added on the GUI, the controller should only have to change if that new button represents some new feature (i.e. opposed to just the same button that existed on screen 1 but didnt exist yet on screen 2, and it is then added to screen 2). There would need to be a corresponding new change in the model also, to support this new functionality/feature. The controller still just has the responsibility of manipulating the domain/model in response to some GUI-driven event.

If the business logic in the model changes due to a bug being fixed, and it requires the controller to change, then this is a special case (or perhaps the model is violating open-closed principal). If the business logic in the model changes to support some new functionality/feature, then that doesnt necessarily impact the controller--only if the controller needs to expose that feature (which would almost always be the case, otherwise why would it be added to the domain model if it wont be used). So in this case the controller has to be modified also, to support manipulating the domain model in this new way in response to some GUI-driven event.

If the controller has to change because, say, the persistence layer is changed from a flat file to a database, then the controller is certainly violating SRP. If the controller always works at the same layer of abstraction, then that can help in achieving SRP.

4

The controller doesn't violate SRP. As you state, its responsibility is to mediate between the models and the view.

That being said, the issue with your example is that you are tying controller methods to logic in the view, ie controller.specificButtonPressed. Naming the methods this way ties the controller to your GUI, you have failed to properly abstract things. The controller should be about performing specific actions, ie controller.saveData or controller.retrieveEntry. Adding a new button in the GUI doesn't necessarily mean adding a new method to the controller.

Pressing a button in the view, means to to do something but whatever that is could easily have been triggered in any number of otherways or even not through the view.

From the Wikipedia article about SRP

Martin defines a responsibility as a reason to change, and concludes that a class or module should have one, and only one, reason to change. As an example, consider a module that compiles and prints a report. Such a module can be changed for two reasons. First, the content of the report can change. Second, the format of the report can change. These two things change for very different causes; one substantive, and one cosmetic. The single responsibility principle says that these two aspects of the problem are really two separate responsibilities, and should therefore be in separate classes or modules. It would be a bad design to couple two things that change for different reasons at different times.

The controller isn't concerned with what is in the view only that when one of its methods is called that it provide specified data to the view. It only needs to know about the model's functionality so far as it knows that it needs to call methods that they will have. It doesn't know anything more than that.

Knowing that an object has a method available to call is not the same as knowing its functionality.

  • 1
    The reason I thought the controller should include methods like specificButtonsPressed() is because I read that the view should know nothing about the functionality of it's buttons and other GUI elements. I've been taught that when a button is pressed, the view should simply report to the controller, and the controller should decide 'what it means' (and then invoke the appropriate methods on the model). Making the view call controller.saveData() means the view has to know about what this button press means, besides the fact that it was pressed. – Aviv Cohn May 4 '14 at 13:18
  • 1
    If I ditch the idea of complete separation between a button press and it's meaning (which results in the controller having methods like specificButtonPressed()), indeed the controller wouldn't be tied to the GUI so much. Should I ditch the specificButtonPressed() methods? Does the advantage I think having these methods has makes sense to you? Or is having buttonPressed() methods in the controller not worth the trouble? – Aviv Cohn May 4 '14 at 13:21
  • 1
    In other words (sorry for the long comments): I think that the advantage of having specificButtonPressed() methods in the controller, is that it separates the View from the meaning of it's button presses completely. However, the disadvantage is that it ties the controller to the GUI in a sense. Which approach is better? – Aviv Cohn May 4 '14 at 13:25
  • @prog IMO The controller should be blind to the view. A button will have some sort of functionality but it doesn't need to know the details of it. It just needs to know that it is sending some data to a controller method. In that regard the name doesn't matter. It can be called foo, or just as easily fireZeMissiles. It will only know that it should report to a specific function. It doesn't know what the function does only that will be calling it. The controller isn't concerned with how its methods are invoked just that it will respond in a certain manners when they are. – Schleis May 4 '14 at 23:09
1

A controllers single responsibility is to be the contract that mediates between the view and the model. The View should be only responsible for the display, the model should only be responsible for the business logic. It's the controllers responsibility to bridge those two responsibilities.

That's all well and good but to venture away from academia a bit; a controller in MVC is generally comprised of many smaller action methods. These actions generally correspond to stuff a thing can do. If I'm selling products, I'll probably have a ProductController. That controller will have actions like GetReviews, ShowSpecs, AddToCart ect...

The View has the SRP of displaying the UI, and part of that UI includes a button that says AddToCart.

The Controller has the SRP of knowing all the Views and Models involved in the process.

The controllers AddToCart Action has the specific SRP of knowing everyone that needs to be involved when an item is added to a cart.

The Product Model has the SRP of modeling product logic, and the ShoppingCart Model has the SRP of modeling how items get saved for later checkout. The User Model has a SRP of modeling the user that's adding stuff to their cart.

You can and should be reusing models to get your business done and those models need to be coupled up at some point in your code. The controller controls each unique way that coupling happens.

0

Controllers actually have only one responsibility: altering the applications state based on user's input.

A controller can send commands to the model to update the model's state (e.g., editing a document). It can also send commands to its associated view to change the view's presentation of the model (e.g., by scrolling through a document).

source: wikipedia

If instead you are having Rails-style "controllers" (which juggle active record instances and dumb templates), then of course out are breaking SRP.

Then again, Rails-style applications are not really MVC to begin with.

protected by gnat Jul 2 '17 at 6:45

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.