dead beef
dead beef

Reputation: 683

Issue with using switch in Java

I can't figure out why it's always returning the value of arg1. I'm building a weight converter.

public double convert(double arg1,int arg2,int arg3) {
    // arg1 = amount, arg2 = from, arg3 = to
    double milligram = 1;
    double gram = 1000;
    double ounce = 28349.5;
    double pound = 453592;
    double answer = 0;
    switch(arg2) {
    case 0: switch(arg3) { // if milligram
            case 0: answer = (arg1 * milligram) / milligram;
            case 1: answer = (arg1 * milligram) / gram;
            case 2: answer = (arg1 * milligram) / ounce;
            case 3: answer = (arg1 * milligram) / pound;
    }
    case 1: switch(arg3) { // if gram
            case 0: answer = (arg1 * gram) / milligram;
            case 1: answer = (arg1 * gram) / gram;
            case 2: answer = (arg1 * gram) / ounce;
            case 3: answer = (arg1 * gram) / pound;
    }
    case 2: switch(arg3) { // if ounce
            case 0: answer = (arg1 * ounce) / milligram;
            case 1: answer = (arg1 * ounce) / gram;
            case 2: answer = (arg1 * ounce) / ounce;
            case 3: answer = (arg1 * ounce) / pound;
    }
    case 3: switch(arg3) { // if pound
            case 0: answer = (arg1 * pound) / milligram;
            case 1: answer = (arg1 * pound) / gram;
            case 2: answer = (arg1 * pound) / ounce;
            case 3: answer = (arg1 * pound) / pound;
    }
    } // end arg2 switch
    return answer;
}

I messed up somewhere in my logic, but I'm failing to see where. Any help would be appreciated.

Upvotes: 1

Views: 159

Answers (5)

akaIDIOT
akaIDIOT

Reputation: 9231

Other than using break, using return statements does the trick as well as it also prevents falling through cases.

case 2: switch(arg3) { // if ounce
    case 0: return (arg1 * milligram) / milligram;

Upvotes: 0

perilbrain
perilbrain

Reputation: 8207

You should be using break in switch statements,else your result may get detoriated

Upvotes: 0

linski
linski

Reputation: 5094

rewrite every case like this

case 2: switch(arg3) { // if ounce
        case 0: answer = (arg1 * ounce) / milligram;break;
        case 1: answer = (arg1 * ounce) / gram;break;
        case 2: answer = (arg1 * ounce) / ounce;break;
        case 3: answer = (arg1 * ounce) / pound;break;

Upvotes: 2

Makoto
Makoto

Reputation: 106470

Nested case statements are pretty hard to read, and tough to debug. It'd be a better idea to encapsulate the functionality you need into a method call instead.

That being said, there's no break statement anywhere in your switch. Regardless of the case, it will fall through to the last case (setting answer equal to the bottom of the case).

Upvotes: 0

Reimeus
Reimeus

Reputation: 159844

You are missing break statements:

case 0: 
   answer = (arg1 * milligram) / milligram;
   break;
   ...

Upvotes: 9

Related Questions