Regarding the minimum use of If Else

Question

Possible Duplicate:
Excessive use of If else staements

I have the below piece of code that make excessive use of if else statement ...

 if (cardType == AARP_CARD_TYPE) {
  userResponse = messageBox.showMessage("CandidateAARPCardAttachCardToExistingTransaction",
      null, IMessageBox.YESNO); // MSG:31.59
  transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
      WalgreensRewardsConstants.ATTACH_CANDIDATE_AARP_CARD);
} else if ((cardType == PSC_CARD_TYPE) && ((!PosHelper.isRunningAsService()))) {
  userResponse = messageBox.showMessage("PendingPSCCardAttachCardToExistingTransaction", null,
      IMessageBox.YESNO); // MSG:31.60
  transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
      WalgreensRewardsConstants.ATTACH_PENDING_PSC_CARD);

} else if ((cardType == DR_CARD_TYPE) && ((!PosHelper.isRunningAsService()))) {
  userResponse = messageBox.showMessage("PendingDRCardAttachCardToExistingTransaction", null,
      IMessageBox.YESNO); // MSG:31.63
  transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
      WalgreensRewardsConstants.ATTACH_PENDING_DR_CARD);

} else if ((cardType == WAG_LOYALTY_CARD_TYPE)){
            transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
              WalgreensRewardsConstants.ATTACH_NOT_ON_FILE);

        if((!PosHelper.isRunningAsService())) {
  userResponse = messageBox.showMessage("CardNotOnFileToAttach", null, IMessageBox.YESNO); // MSG:31.32
  // BUC
  // 1.22.1
}


} else { // If the device is neither of these, POS displays Message 1
  // Button, MSG 31.14. [BUC
  // 1.23.2]
  displayMessage("InvalidLoyaltyCard");
  transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
      NOT_VALID_LOYALTY_CARD);
  userResponse = -1;
}

I want o remove the excessive use of if else staement soI canged a little bit in this below form

byte cardType ;

    switch (cardType) {
    case  AARP_CARD_TYPE:
         userResponse = messageBox.showMessage("CandidateAARPCardAttachCardToExistingTransaction",
                  null, IMessageBox.YESNO); // MSG:31.59
              transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
                  WalgreensRewardsConstants.ATTACH_CANDIDATE_AARP_CARD);
                    // blah
        break;

    case PSC_CARD_TYPE:
        if ((!PosHelper.isRunningAsService())
                {

             userResponse = messageBox.showMessage("PendingPSCCardAttachCardToExistingTransaction", null,
                      IMessageBox.YESNO); // MSG:31.60
                  transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
                      WalgreensRewardsConstants.ATTACH_PENDING_PSC_CARD);
                }   
        // blah
        break;

        case  DR_CARD_TYPE :                
            if ((!PosHelper.isRunningAsService())
                    {                   
                userResponse = messageBox.showMessage("PendingDRCardAttachCardToExistingTransaction", null,
                          IMessageBox.YESNO); // MSG:31.63
                      transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
                          WalgreensRewardsConstants.ATTACH_PENDING_DR_CARD);
            // blah
            break;


                    case   WAG_LOYALTY_CARD_TYPE :

                        transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
                                  WalgreensRewardsConstants.ATTACH_NOT_ON_FILE);
                        if((!PosHelper.isRunningAsService())) {
                              userResponse = messageBox.showMessage("CardNotOnFileToAttach", null, IMessageBox.YESNO); // MSG:31.32
                              // BUC
                              // 1.22.1
                            }
                        // blah
                        break;                  


    // ...

    default:
         displayMessage("InvalidLoyaltyCard");
          transaction.setValue(ITransactionHashtableWag.LOYALTY_MESSAGE_DISPLAYED,
              NOT_VALID_LOYALTY_CARD);
          userResponse = -1;
        break;
    }

Please advise is it correct..!!

When you run the code, does it do what you want it to do? That is, it performs properly but you just want to know if this is a good coding approach? — DOK, May 23 '12 at 17:36
You should edit your previous question and ask for doubt clarifications instead of posting a new question altogether! — Kazekage Gaara, May 23 '12 at 17:52

score 2 · Answer 1 · edited May 23 '17 at 10:09

2

instead of a if else statements or switch case I prefer to use mapping of keys to delegates. You could create a hashmap that holds all the delegates and just call the right one. see this StackOverflow question about delegating.

this will give you a clean code of

PointersToMethods.get(key).call();

edited May 23 '17 at 10:09

Community

1
1

answered May 23 '12 at 17:46

Horonchik

477
2
11

That would be a good answer to the [original question](http://stackoverflow.com/questions/10724506/excessive-use-of-if-else-staements). – assylias May 23 '12 at 17:48

score 0 · Answer 2 · answered May 23 '12 at 17:36

0

Does it work? It certainly looks a lot cleaner the way you have modified it

answered May 23 '12 at 17:36

ControlAltDel

33,923
10
53
80

score 0 · Answer 3 · answered May 23 '12 at 17:36

0

The 2 methods are not equivalent. In the first case, if (cardType == PSC_CARD_TYPE && PosHelper.isRunningAsService()) then the code goes to the default else clause, whereas in the second one, it does nothing (break;).

ps: don't you have unit tests to run to see if it works as expected?

answered May 23 '12 at 17:36

assylias

321,522
82
660
783

@assylias..It woulbe a great help if you could write and make modifications in my second case so that It becomes equilavent to first one – user1406575 May 23 '12 at 17:54
@user1406575 I think it is fairly clear: in the case where `cardType == PSC_CARD_TYPE` and `PosHelper.isRunningAsService() == true`, your first code goes to the else whereas your second does not go to default. – assylias May 23 '12 at 17:56
@assylias..Please could you correct my second approach by writing complete code as I have done with correct implementations ...that would be a great help..thanks inadvance – user1406575 May 23 '12 at 17:58
@user1406575 I think you need to learn a few things about StackOverflow (this is a friendly comment). 1. [You can't expect users to write your code for you](http://meta.stackexchange.com/a/128553/179508). 2. I have already answered to your previous question that this approach is messy. Now you see why. 3. It is good practice to [accept answers that help you](http://meta.stackexchange.com/a/5235/179508) and I suggest you go back to your previous question and see if this is applicable. – assylias May 23 '12 at 18:18
@user1406575 Finally, once you follow the sites guidelines, you will find that some users are extremely knowledgeable and helpful. – assylias May 23 '12 at 18:19
@user1406575 And I forgot, as pointed out in the comments above, you have the option to edit you questions to refine them. So this question should have been an edit of the original one. ps: welcome to SO! ;-) – assylias May 23 '12 at 18:24

Regarding the minimum use of If Else

3 Answers3