Dmitry Senkovich Dmitry Senkovich -3 years ago 139
reST (reStructuredText) Question

Pros and cons of having one controller for multiple resources

We have a Spring MVC application, most of the REST operations are the only GET operation per resource. So currently we have many controllers with the only GET method that doesn't differ from each other (even in urls, content types, parameters and etc).

In order to remove such a duplication our team mate offers to make one controller with the only GET operation and a map with service (resource name -> resource service).

But we see such downsides as more complicated Spring injection tuning, no opportunities to add some restrictions on content types, parameters - customize operation in one word. Additionally, there are several resources that are to reside in a separate controller. Also I don't expect there is at least easy way to document the only method in Swagger in multiple ways (have different description).

So as for me on the one side is less code but on the other are restricted opportunities of operation customization, mix of architectures, lack of proper documentation or at least complicated configuration. I don't think it is a good approach here to make one method.

Am I right? If so how can I prove it. If not why? Thank you for you time and ideas!

Answer Source

Yes, you are right. In short, according to single responsibility principle each controller should do only one job (process only one URL).

You perfectly describe problems which generic controller would deal. Also think about what if some controller perfectly fits to generic rule right now but require specific stuff next month? You have to copy-paste code and then add new. So after some time you get mess with huge and complex generic controller and duplicate code. Nobody could predict how fast it could be since business requirements could be added unexpectedly for developer team.

On the other hand your teammate is right in his desire to reduce duplicate code. At least not all developers want to spend their time in making code more clean. And most people need to to gain recognition (ensure their opinion has the value). So don't send him away :)

What I could recommend: Introduce abstract parent and use inheritance and Template pattern for similar controllers

/** Interface mainly works as a marker. 
  At first look, interface isn't necessary but it'll improve maintainability.
  Next year you say 'thank you' to yourself  */
interface IController { 
  //some methods which will implement EACH controller even the most specific
  public void doGet(params)
abstract class AbstractController implements IController {
 /** Class implements default behavior for controllers.
     Implementation written so child classes could adopt behaviour easily*/
  public void doGet(params) {
    // use Template pattern
  // common stuff which should be done at first
  protected void doLog(params) { // your favorite logger here}
  // extension point for inherited classes
  abstract protected void prepareStuff();
  // here is the real processing for default controller
  public void process() {
    //implement common processing for GET request
  // Prefer to use protected method instead of field
  protected String getURL() { return DEFAULT_URL;}
// usual controller has nothing special
class Controller1 extends AbstractController {
  protected String getURL() { return "url1";}
  protected prepareStuff() {// do nothing}
// do some specific preparation/checks
class Controller2 extends AbstractController {
  protected prepareStuff() {//preparation/checks here }
  /** Note I 'forget' to override getURL() so it'll process DEFAULT_URL.
   It could be useful if AbstractController calculates url dynamically and
   you don't want to write boilerplate strings "/myApp/section7".
   Also you could write abstract getURL()
/** custom controller but you want to re-use some common code. 
In fact I would not recommend this way as usual approach */
class Controller3 extends AbstractController {
  /** Overriding this method totally discards the Template pattern. 
      It could (and will) lead to confusing and errors*/
  public void doGet(params) { // new implementation }
  protected prepareStuff() {
    // you don't need it but you have to override since it abstract
// totally custom controller. Implements interface just as a marker
class SpecificController implements Controller {
  // In order to support legacy code just call method wich has been already written. You even no need to rename it.
  public void doGet(params) { specificMethod();}
  // lagacy method which probably is used somewhere else
  public void specificMethod() {  // the actual logic here}

In fact I supposed similar solution in a project. Using IDE functions like 'introduce method' and 'move to the parent' I refactored dozens of classes in a single day.

Hope you or your teammate could implement and compare such idea in couple of days

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download