CODEWITHSUNDEEP
javaif-statement
Neeti
Neeti

Reputation: 397

Cleaner way for if-else condition

I have a method having multiple if-else conditions (which is growing with each new msg-type support)

public Message<?> doTransform(Message<String> message) throws TransformationException {
        
        try {
            MessageBuilder<String> messageBuilder = null;
            String payload = message.getPayload();
            String payloadSubStr = payload.substring(0, Math.min(payload.length(), 100));
            
            
            if(payloadSubStr.contains("<Management>")){
                messageBuilder = buildManagementMsg(message);
            } else if (payloadSubStr.contains("<Administration>") || (payloadSubStr.contains("<OtherAdministationAlert>"))){
                messageBuilder = buildAdminMessages(message);
            } else if (payloadSubStr.startsWith("Council")){
                messageBuilder = parseCouncilMessages(message);
            } else if (payloadSubStr.indexOf("Security") >= 0
                    ||  payloadSubStr.indexOf("OtherSecurityAlert") >= 0){
                messageBuilder = buildSecurityMessages(message);
            } else if ( payloadSubStr.indexOf("<Staff>") >= 0
                    ||  payloadSubStr.indexOf("<OtherStaffAlert>") >= 0){
                messageBuilder = buildStaffMessages(message);
            } else if(payloadSubStr.indexOf("<Student>") >= 0) {
                messageBuilder = buildStudentMessages(message);
            }else {
                messageBuilder = buildOtherMessages(message);
            }
            
            return messageBuilder.build();
            
        } catch(Exception e) {
            throw new TransformationException(e);
        }
    }
  1. Reason for doing substring: To avoid complete msg traversing with each if/else condition
  2. Reason for using contains/indexOf combination: Messages received by this method can vary

Wanted to replace these if/else statements with some more cleaner logic. Not getting if Switch/Enum can be used or need to use any pattern as suggested over https://www.baeldung.com/java-replace-if-statements. Gone through various similar questions available but not getting anything. Any suggestion will be helpful.

Thanks

Upvotes: 0

Views: 394

Answers (2)

Thomas
Thomas

Reputation: 88707

If you are concerned with the number of conditions you'd need to add you could introduce a list of "message builder factories" (for lack of a better name atm) that you could append to.

A factory would contain a predicate to test the substring for and a messageBuilder(...) method. Then you'd iterate over the factory, check each of the predicates and if execute messageBuilder() on the first that matches.

Example:

interface MessageBuilderFactory<T> {
  boolean test(T payload);
  MessageBuilder<T> messageBuilder(Message<T> message);
}

class ManagementMBFactory implements MessageBuilderFactory<String> {
   boolean test(String payload) {
     return payload.contains("Management");
   }

   MessageBuilder<String> messageBuilder(Message<String> message) {
     //content of buildManagementMsg() here
   }
}

And in your code:

List<MessageBuilderFactory<String>> factories = ... //get the list of factories from somewhere

for( MessageBuilderFactory<String> factory : factories) {
  if( factory.test(payloadSubStr) {
    messageBuilder = factory.messageBuilder(message);
  }
}

An advantage of doing it that way would be that the list of possible message builder factories is easily available and classes can be kept small (not all those buildXxx() methods in one single class).

Alternatively, if your message payload allows for that, you could actually try to parse it (it looks like XML) and operate on events, i.e. elements being found. That might be faster in the case of many small payloads and a huge number of possible message builders.

Upvotes: 3

geco17
geco17

Reputation: 5294

People are divided on the idea of multiple return statements in java but in this case I think I would tend to do that:

        if(payloadSubStr.contains("<Management>")){
            return buildManagementMsg(message);
        }
        if (payloadSubStr.contains("<Administration>") || (payloadSubStr.contains("<OtherAdministationAlert>"))) {
            return buildAdminMessages(message);
        }
        if (payloadSubStr.startsWith("Council")){
            return parseCouncilMessages(message);
        }
        if (payloadSubStr.indexOf("Security") >= 0
                ||  payloadSubStr.indexOf("OtherSecurityAlert") >= 0){
            return buildSecurityMessages(message);
        }
        if ( payloadSubStr.indexOf("<Staff>") >= 0
                ||  payloadSubStr.indexOf("<OtherStaffAlert>") >= 0){
            return buildStaffMessages(message);
        }
        if (payloadSubStr.indexOf("<Student>") >= 0) {
            return buildStudentMessages(message);
        }
        return buildOtherMessages(message);

Going one step further this could be done with a validation service.

public class ValidationService {
    public boolean isManagement(String str) { return str.contains("<Management>"); 
    // ... and so on
}

And you can inject the service into the code such that you have

if (validationService.isManagement(payloadSubStr)) {
    return buildManagementMsg(message);
}
// ...

For a case with conditions currently in OR you could use a list in the service, for example

public boolean isSecurity(String str) {
    for (String term : new String[]{"Security", "OtherSecurityAlert"}) {
        if (str.contains(term)) {
            return true;
        }
    }
    return false;
}

Upvotes: 1

Related Questions