Keeping your code clean by sweeping out “if” statements

Keeping your code clean by sweeping out “if” statements

Originally posted on dev

One of the most common things I see around that I think makes code more difficult to read is the overuse of “if” statements. It’s one of the first programming tools we learn, and it is usually when we learn that the computer can do pretty much anything we like, “if” we use those statements right. Following usually are long ours of printfs and debugging and trying to figure out why the program isn’t getting into that third nested “if” as we’re sure it would do! That’s how many of us have approached programming, and that’s what many of us have grown used to.

As I studied more about code design and best practices I began noticing how using lots of if statements could be an anti-pattern, making the code worse to read, debug and maintain. And so out I went to find alternative patterns to those if statements, and I think the patterns I’ve found have improved my code’s readability and maintainability.

Of course there’s no silver bullet and any pattern should be used where it makes sense. I think it’s important for us developers to have options when attempting to write good quality code.

To illustrate this point I’ll share two simple examples, and I hope to hear your thoughts on the matter in the comments.

First let’s see the example where we have to handle an incoming message from outside our application boundaries, and the object contains a string property specifying a type. It might be an error type from a mass email marketing, and we would have to translate that to our own domain’s error type.

Usually I see that implemented like this:

    private ErrorType translateErrorType(String errorString) {
        if ("Undetermined".equals(errorString)) {
            return UNKNOWN_ERROR;
        } else if ("NoEmail".equals(errorString)) {
            return INVALID_RECIPIENT;
        } else if ("MessageTooLarge".equals(errorString)) {
            return INVALID_CONTENT;
        } else if ("ContentRejected".equals(errorString)) {
            return INVALID_CONTENT;
        } else if ("AttachmentRejected".equals(errorString)) {
            return INVALID_CONTENT;
//      } else if (...)

        } else {
            throw new IllegalArgumentException("Error type not supported: " + errorTypeString);
        }
    }

You see where this is going, right? It could be a switch statement, and it would be just as cluttered, with much more language specific words than actual business domain language.

There are a number of approaches to getting rid of this kind of “if” entanglement, but I guess the simplest one is the use of a map.

    public class ErrorTypeTranslator {

        private final static Map<String, ErrorType> errorTypeMap;

        static {
            errorTypeMap = Map.of(
            "Undetermined", UNKNOWN_ERROR,
            "NoEmail", INVALID_RECIPIENT,
            "MessageTooLarge", INVALID_CONTENT,
            "ContentRejected", INVALID_CONTENT,
            "AttachmentRejected", INVALID_CONTENT
//          (…)
            );
        }

        public ErrorType translateErrorType(String errorTypeString) {
            return requireNonNull(errorTypeMap.get(errorTypeString),
                    () -> "Error type not supported: " + errorTypeString 
        }
    }

See the difference? Now the business logic is front and center, and any developer approaching this class should be able to very easily understand what it does and change it should it be needed. Lots of language specific words and symbols have disappeared making way for a much nicer, cleaner and less error prone code.

This kind of pattern is also very good for simple factories, where you can have a map of singletons, or even suppliers as values. For example, let’s say you have to return a handler bean based on an enum retrieved from the database.

What I usually see is something like:

    public class IntegrationHandlerFactory {

        private final EmailIntegrationHandler emailHandler;
        private final SMSIntegrationHandler smsHandler;
        private final PushIntegrationHandler pushHandler;

        public IntegrationHandlerFactory(EmailIntegrationHandler emailHandler,
                              SMSIntegrationHandler smsHandler,
                              PushIntegrationHandler pushHandler) {
            this.emailHandler = emailHandler;
            this.smsHandler = smsHandler;
            this.pushHandler = pushHandler;
        }

        public IntegrationHandler getHandlerFor(Integration integration) {
            if (EMAIL.equals(integration)) {
                return emailHandler;
            } else if (SMS.equals(integration)) {
                return smsHandler
            } else if (PUSH.equals(integration)) {
                return pushHandler
            } else {
                throw new IllegalArgumentException("No handler found for integration: " + integration);
            }

        }
    }

Let´s try using a Map instead:

    public class IntegrationHandlerFactory {

        private final Map<Integration, IntegrationHandler> handlerMap;

        public IntegrationHandlerFactory(EmailIntegrationHandler emailHandler,
                              SMSIntegrationHandler smsHandler,
                              PushIntegrationHandler pushHandler) {

            handlerMap = Map.of(
                        EMAIL, emailHandler,
                        SMS, smsHandler,
                        PUSH, pushHandler
            );
        }

        public IntegrationHandler getHandlerFor(Integration integration) {
            return requireNonNull(handlerMap.get(integration),
                            () -> "No handler found for integration: " + integration);
        }

    }

Much neater, isn’t it? Once again a very simple design that gets rid of many if / else if statements and makes it very easy to add new options.

Source: dev