current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

I was playing around with Java Card and I try to do one of the examples.

P1 is what it has to do (e.g. decrypt, encrypt, etc) and P2 is to select which key to use in P1. Here is what the code looks like - well, half of it anyway, since I'm not really used to formatting in this site and it takes awhile to copy the code and make it pretty.

So my question here is, how can I simplify this nested switch? Or maybe I don't need to use nested switch at all?

switch (buf[ISO7816.OFFSET_P1]){
    case (byte) 0x01:
        switch (buf[ISO7816.OFFSET_P2]){
            case (byte) 0x01:
                doSingleDES(apdu, DESKey1);
                return;
            case (byte) 0x02:
                doSingleDES(apdu, DESKey2);
                return;
            case (byte) 0x03:
                doSingleDES(apdu, DESKey3);
                return;
            case (byte) 0x04:
                doSingleDES(apdu, DESKey3);
                return;
            default:
                ISOException.throwIt(ISO7816.SW_WRONG_P1P2);
        }return;
    case (byte) 0x02:
        switch (buf[ISO7816.OFFSET_P2]){
            case (byte) 0x01:
                doEncrypt(apdu, DESKey1);
                return;
            case (byte) 0x02:
                doEncrypt(apdu, DESKey2);
                return;
            case (byte) 0x03:
                doEncrypt(apdu, DESKey3);
                return;
            case (byte) 0x04:
                doEncrypt(apdu, DESKey4);
                return;
            default:
                ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
    }return;
    default:
            ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
share|improve this question

migrated from stackoverflow.com Dec 27 '13 at 4:42

This question came from our site for professional and enthusiast programmers.
    
Since it looks like your code is working, I'm going to send you in the direction of the friendly folks at Code Review. –  Michael Myers Dec 27 '13 at 4:42
1  
I don't know very much about Java Card, but what is the difference between SW_WRONG_P1P2 (thrown in first inner switch) and SW_INCORRECT_P1P2 (thrown in default and second inner switch)? –  ssssteffff Dec 27 '13 at 9:05
    
That's what I thought too when I saw them in the auto-completion list. I just try them both to see the difference. SW_WRONG_P1P2 return "Wrong parameter" and SW_INCORRECT_P1P2 return "Incorrect parameter" if you enter 05 for both case. –  Archie Dec 27 '13 at 9:29

2 Answers 2

up vote 6 down vote accepted

I would focus on eliminating the inner switch statements. You could try to eliminate the outer switch as well, but it's probably not worthwhile, especially for a resource-constrained environment.

byte[][] desKeys = new byte[][] {
    null, DESKey1, DESKey2, DESKey3, DESKey4
};

try {
    switch (buf[ISO7816.OFFSET_P1]) {
      case 0x01:
        doSingleDES(apdu, desKeys[buf[ISO7816.OFFSET_P2]]);
        return;
      case 0x02:
        doEncrypt(apdu, desKeys[buf[ISO7816.OFFSET_P2]]);
        return;
      default:
        ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
    }
} catch (ArrayIndexOutOfBoundsException badP2) {
    ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
}
share|improve this answer
    
I try to use your code, but it gives me error because it says can't convert from byte [] to byte [][]..if I try eliminate one of the bracket, it gives me error because can't convert null to byte..if I were to add another bracket to the single bracket, the package is giving me error because of invalid type. –  Archie Dec 27 '13 at 7:10
    
@Archie what is the type of DESKey1, DESKey2, etc? If its long, try long[] desKeys = new long[] {0, DESKey1, DESKey2, DESKey3, DESKey4}; –  abuzittin gillifirca Dec 27 '13 at 9:48
    
@abuzittingillifirca I use byte[]..maybe it's something to do with the version of the java card I'm using? I'm using 2.2 version btw. –  Archie Dec 27 '13 at 9:57
    
@abuzittingillifirca Thanks for the fix. –  200_success♦ Dec 27 '13 at 10:00
up vote -1 down vote

You could use the following construct to switch on both P1 and P2 simultaneously:

switch(Util.getShort(buf, ISO7816.OFFSET_P1)) {
    case (short)0x0101:
        doSingleDES(apdu, DESKey1);
        return;
    case (short)0x0102:
        doSingleDES(apdu, DESKey2);
        return;
    case (short)0x0103:
        ...
    case (short)0x0201:
        doEncrypt(apdu, DESKey1);
        return;
        ...
    default:
        ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
        return; // not-reachable
}

If you insist on different SW for P1=0x01 the default case might look like this:

    default:
        if(buf[ISO7816.OFFSET_P1]==(byte)0x01) {
            ISOException.throwIt(ISO7816.SW_WRONG_P1P2);
        } else {
            ISOException.throwIt(ISO7816.SW_INCORRECT_P1P2);
        }
        return; // not-reachable
share|improve this answer
1  
You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and how it improves upon the original) so that the author can learn from your thought process. –  Quill Jul 17 at 10:52
    
Have edited the post. Do you think it is better now? –  vlp Jul 17 at 16:02
    
You should improve the post by explaining how your solution is better, rather than simply providing an alternative solutions. –  Quill Jul 18 at 0:47
    
As written in the first paragraph: This solution could be better as it does the switch on both values simultaneously. –  vlp Jul 18 at 1:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.