Is an antipattern returning differente objects in a single rest method?

Question

I have seen around the Internet several rest web services with the following behaviour. In case there are any errors, they return a Error object, otherwise they return, say, MyClass.

See the following example...

/*
 * Data transfer objects
 */
@Data
public class Error {
  private Integer id;
  private String description;
}

@Data
public class MyClass{
  // Whatsoever attribute...
}

/*
 * Web service (code omitted and abbreviated for the sake of simplicity)
 */
@PostMapping("myclass")
public ResponseEntity<MyClass> createMyClass(
    @RequestBody MyClass pMyClass
) {
    Optional<MyClass> optMyClass = facade.createMyClass(pMyClass);
    if(optTrip.isEmpty())
        Error err = Error.Builder().id(400).description("Any reason...").build();
        return ResponseEntity.ok(err); //may return the same HTTP 400 error instead of 200 Ok
    else
        return ResponseEntity.ok(optMyClass.get());
}

I feel that this is an antipattern for two major reasons. The first one is that the web method is not returning one kind of class objects, usually writing T or ? as returning class. The second one is that they are taking responsability of returning any error it may occur, instead of taking advantage of using HTTP headers and answers themselves.

Do you agree with me?

Answer 1

You might think of it as an anti-pattern, yes. Consider this definition for example:

According to the authors of Design Patterns, there must be at least two key elements present to formally distinguish an actual anti-pattern from a simple bad habit, bad practice, or bad idea:

1. A commonly used process, structure, or pattern of action that despite initially appearing to be an appropriate and effective response to a problem, has more bad consequences than good ones.
2. Another solution exists that is documented, repeatable, and proven to be effective.

Your example respects both of these points.

First of all, when you call a REST method, you might get an exception or error. You need to return a response to the client, and because you can't build the actual response object ('cause the method failed in some way), you have to return an error object. This is the right thing to do, but the problem is that the method that builds the normal response object also takes care of building the error object. This breaks the principle of Separation of Concerns. The method is concerned with error handling and error response generation too. This seems to be the appropriate and effective response to the problem but in time causes problems because all of your code is littered with all sorts of pieces that build error objects instead of actual response objects.

An now, point 2 of the above definition comes into play. There is another solution that exists that is documented, repeatable, and proven to be effective. It's been a while since I've written Spring REST services, but I do remember that you had good options for exception handling (HandlerExceptionResolver, @ExceptionHandler, etc) that you could use to cleanly handle exceptions and errors in a global and reusable manner. All that code for handling and building error objects moved outside of the methods themselves and the concerns were separated. Much cleaner, much more maintainable.

So with these two points, you might think at the example you are showing as an anti-pattern. There isn't an authority anywhere to actually confirm or infirm this naming, but more experienced developers can definitely tell you it's a bad idea and practice to handle errors like that in REST services.

If you are learning to build Spring REST services and see Internet examples with that behavior in them, consider also if they are not simple examples focusing on teaching you concepts piece by piece. At some point you might discover more advanced examples that may say "Remember when we called the method and we built error objects to return? That was not a really good way of doing it. Let's explore a better alternative. Keep reading...", etc.