When we have package by feature, do we need layers?
A case for layers with a sprinkle of other learnings mixed in
java.lang.IllegalArgumentException: No enum constant found could occur in a backend when the client sent in an String representing a certain command subtype that did not map to any known subtype. This application did not really do layering so it made sense to the developers to catch this Exception as they did with various other Exceptions, and directly translate it to a custom Exception. That custom Exception was later mapped to a response code.
EnumNotFoundException extends BizAppException
This application caught all BizAppExceptions (called something else) in a Filter and translated to a http response code with a message depending on the type of Exception. As the one reviewing this, I saw three problems with it.
Voicing over the original Exception
For them, EnumNotFoundException was clear. For me or others who may patch this application on an production issue, but rarely hack on it otherwise, we would have preferred java.lang.IllegalArgumentException: No enum constant found. There is really no reason to obscure data and trying to clarify an error like this is misguided and causes obfuscation. What could have been done is translate to a business problem. The result from running the command, could include a enum ResultType with ResultType.OK and ResultType.NOT_OK_UNKNOWN_SUBTYPE. That would have been documenting the issue in a much better way should the business method return the NOT_OK status.
Hiding the transformation from business problem to http response
First off, hard coding if EnumNotFoundException is a client issue or a server issue would never scale. In their use case, it was a client problem according to them. However the next developer doing a feature could see this as a server issue, if for example the enum-constant-to-be may be from the server originally so if the server does not recognise it, its a server issue. The abstraction used is already leaking badly and maps really poorly to the actual data transformation problem we had originally. Instead proper layering should occur and the client that interacted with the business method should be responsible for mapping to whatever message is suitable.
ResultType type = businessUsecase.runSubcommand(String subcommand)if (type == ResultType.NOT_OK_UNKNOWN_SUBTYPE)
throw new ClientException(400, ResultType.NOT_OK_UNKNOWN_SUBTYPE.toString())//code above is simplified
When you as a stranger to the code has identified the endpoint with an issue and you see this, you know exactly what is going on. Why would we want an hidden mapping in some Filter? Transfer between layers should be explicit.
Creating god classes and strong coupling
We don’t want the same exception for otherwise unrelated features. We probably don’t want custom exceptions at all, but if we did, they should speak business and not technology. The common exceptions and common Filter was just one big source of coupling in this application.
Solving data transformation problems is the job. When that is confused with doing “smart coding” and trying to make things generic, we end up with indirect code. Don’t be afraid to make the code do something, in fact that is the point, it should solve something and do something directly rather than indirectly.