CODEWITHSUNDEEP
javaperformance
gmhk
gmhk

Reputation: 15940

Code clean up for if else statement

Here is the peace of code, Which I am looking to make it more efficient I see lots of if else statements,

Can any one help me to make it simple and reduce the number of lines of code

if(mmPageCounter.equalsIgnoreCase("trueNotFull")){
    if(whatPageNext.equalsIgnoreCase("first"))
        accountListSize = 5;
    else
        accountListSize = acctIdList.size();
}else if(mmPageCounter.equalsIgnoreCase("true")){
    if(whatPageNext.equalsIgnoreCase("first"))
        accountListSize = 5;
    else
        accountListSize = 10;
}else if(mmPageCounter.equalsIgnoreCase("false")){
    if(whatPageNext.equalsIgnoreCase("alwaysFirst"))    
        accountListSize = acctIdList.size();
}

In the above code based on the mmPageCounter based on the value I am setting the index of the For loop either to the full capactiy, or to the actual value

Full capacity means, The for loop will iterate only 10 items

ps : I have got the code working and its fine, But i am looking only for the fine tuning

If any one one has any fine tuning tips please share the links.

Upvotes: 0

Views: 3098

Answers (4)

Miquel
Miquel

Reputation: 4829

This does not make it faster but reduces code, you can substitute:

if(whatPageNext.equalsIgnoreCase("first"))
    accountListSize = 5;
else
    accountListSize = 10;

by:

accountListSize = whatPageNext.equalsIgnoreCase("first") ? 5 : 10;

and:

if(whatPageNext.equalsIgnoreCase("first"))
    accountListSize = 5;
else
    accountListSize = acctIdList.size();

by:

accountListSize = whatPageNext.equalsIgnoreCase("first") ? 5 : acctIdList.size();

UPDATE:

If mmPageCounter can only be "trueNotNull", "true", or "false" then I think this code is equivalent:

int maxsize = mmPageCounter.equalsIgnoreCase("true") ? 10 : acctIdList.size();

if(mmPageCounter.equalsIgnoreCase("false")){
    if(whatPageNext.equalsIgnoreCase("alwaysFirst"))    
        accountListSize = accountListSize = acctIdList.size();
} else {
  accountListSize = whatPageNext.equalsIgnoreCase("first") ? 5 : maxsize
}

but you should write tests against current code and check that this code behaves exactly in the same way on edge cases.

Upvotes: 1

teodozjan
teodozjan

Reputation: 913

I would write it reversely. When account size should be set to 10? When account size should be set to 5?

if(shouldBeFive()){
  accountListSize = 5;
} else if (shouldBeTen()){
  accountListSize = 10;
} else{
  accountListSize = acctIdList.size()
}

Also you can switch-case in java7.

Upvotes: 3

Costi Ciudatu
Costi Ciudatu

Reputation: 38195

My suggestion is not to make your code shorter, but more extensible and easier to maintain. It will however make the client code a lot shorter.

I would choose an enum but without a switch statement, as I can spot a strategy here and I'd prefer to encapsulate the logic outside the client. Also, I'd provide an abstraction (an interface) that would allow me or others to come with alternative implementations in the future, without adding new constants to the enum. The literal values for your strings and numbers I'd define as constants (in the abstraction, if that makes sense)

Here's the "strategy" abstraction:

interface PageCounter {
    String FIRST = "first";
    String ALWAYS_FIRST = "alwaysFirst";

    int FIRST_SIZE = 5;
    int DEFAULT_SIZE = 10;

    int getAccountListSize(String whatPageNext, List<?> acctIdList);
}

Here's the enum implementing the PageCounter interface and providing the default implementations:

enum DefaultPageCounter implements PageCounter {
    TRUENOTFULL {
        @Override
        public int getAccountListSize(String whatPageNext, List<?> acctIdList) {
            return FIRST.equalsIgnoreCase(whatPageNext) ? FIRST_SIZE : acctIdList.size();
        }
    },
    TRUE {
        @Override
        public int getAccountListSize(String whatPageNext, List<?> acctIdList) {
            return FIRST.equalsIgnoreCase(whatPageNext) ? FIRST_SIZE : DEFAULT_SIZE;
        }
    },
    FALSE {
        @Override
        public int getAccountListSize(String whatPageNext, List<?> acctIdList) {
            // some default (non)value is required here: -1 can be whatever.
            return ALWAYS_FIRST.equalsIgnoreCase(whatPageNext) ? acctIdList.size() : -1;
        }
    };
}

Here's your code (a lot shorter in the client, and unaware of the existing possibilities)

PageCounter counter = DefaultPageCounter.valueOf(mmPageCounter.toUpperCase());
int accountListSize = counter.getAccountListSize(whatPageNext, acctIdList);

The counter can be provided by some factory method so that you can switch the PageCounter implementation for subclasses (that's why I chose to use an interface after all), but I didn't want to make this more complicated than it already is.

Upvotes: 1

Eser Ayg&#252;n
Eser Ayg&#252;n

Reputation: 8004

If you know all possible values of mmPageCounter and whatPageNext beforehand, you can use a HashMap.

During initialization:

accountListSizes = new HashMap<String, Integer>();
accountListSizes.put("trueNotFull|first", 5);
accountListSizes.put("true|first", 5);
...

During computation:

String key = String.format("%s|%s", mmPageCounter, whatPageNext);
Integer value = accountListSizes.get(key);
if (value == null) {
    accountListSize = acctIdList.size();
} else {
    accountListSize = value;
}

This implementation is a little bit slower than yours, but it is much easier to read and extend. You can alter the mapping even at run-time.

Upvotes: 0

Related Questions