How do I solve repeated code caused by dialogue boxes on my switch case method?

Question

        public void onClick(View view)
        {
            double p,d;
            try {
                switch (view.getId()) {
                    case R.id.button:
                        p = Float.parseFloat(e.getText().toString());
                        d = p * 1.41;
                        e.setText("£" + p + " = " + "$" + d);
                        adb.setTitle("£ to $");
                        adb.setMessage("£" + p + " = " + "$" + d);
                        adb.show();
                        break;
                    case R.id.button2:
                        p = Float.parseFloat(e.getText().toString());
                        d = p * 1.14;
                        e.setText("£" + p + " = " + "€" + d);
                        adb.setTitle("£ to €");
                        adb.setMessage("£" + p + " = " + "€" + d);
                        adb.show();
                        break;
                }
            }
            catch (NumberFormatException e)
            {
                d = 0.0;
            }
        }
    };

On my code i have adb.setMessage which repeats what e.setText("£" + p + " = " + "$" + d); has in brackets. Is there any way to solve this issue?

I also have a problem where adb.show(); is repeated and would love to know how I can solve this. Thanks!

I suggest you use the Android Studio debugger to step through your code to see what it is doing. Read https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems for tips. — Code-Apprentice, Jan 29 '18 at 20:32
@Code-Apprentice Maybe my question wasnt clear sorry, I just wanted to know if there is a more efficient way of implementing dialogue boxes in my switch case as there is repeated code — Dev, Jan 29 '18 at 20:39
okay, I was confused if you were asking about repetition in your output vs repetition in the actual code. — Code-Apprentice, Jan 29 '18 at 22:57

score 2 · Accepted Answer · answered Jan 29 '18 at 20:42

There are only 2 differences, so assign those to variables, and share the rest:

public void onClick(View view) {
    String currency;
    double rate;
    switch (view.getId()) {
        case R.id.button:
            currency = "$";
            rate = 1.41;
            break;
        case R.id.button2:
            currency = "€";
            rate = 1.14;
            break;
        default:
            throw new UnsupportedOperationException("Unknown button");
    }
    try {
        double p = Double.parseDouble(e.getText().toString());
        double d = p * rate;
        e.setText("£" + p + " = " + currency + d);
        adb.setTitle("£ to " + currency);
        adb.setMessage("£" + p + " = " + currency + d);
        adb.show();
    } catch (NumberFormatException e) {
        e.setText("Not a valid number: " + e.getText());
    }
}

I fixed the copy/paste error in adb.setMessage where it said $ for button2.

I also fixed the number parsing to use Double.

I didn't think to do it this way, its easier to read too. Thanks a lot! — Dev, Jan 29 '18 at 20:47

Code-Apprentice · Answer 2 · 2018-01-29T23:05:52.803

Here the differences between the two cases is only in values used. And those values are hardcoded literals. Both of these clues suggest that you can refactor the common code into a method:

public void onClick(View view) {
    switch (view.getId()) {
        case R.id.button:
            showCurrency("$", 1.41);
            break;
        case R.id.button2:
            showCurrency("€", 1.14);
            break;
        default:
            throw new UnsupportedOperationException("Unknown button");
    }
}

private void showCurrency(currency, rate) {
    try {
        double p = Double.parseDouble(e.getText().toString());
        double d = p * rate;
        e.setText("£" + p + " = " + currency + d);
        adb.setTitle("£ to " + currency);
        adb.setMessage("£" + p + " = " + currency + d);
        adb.show();
    } catch (NumberFormatException e) {
        e.setText("Not a valid number: " + e.getText());
    }
}

Additional suggestions

Personally, I prefer giving each button its own handler, rather than using a switch statement in a single handler:

public void onClick1(View view) {
    showCurrency("$", 1.41);
}

public void onClick2(View view) {
    showCurrency("€", 1.14);
}

This only works if you set the onClick handler in XML and these methods are in an activity. If you are calling setOnClickListener() in onCreate(), then you will need to create two anonymous inner classes instead of implementing the OnClickListener interface in your Activity class.

How do I solve repeated code caused by dialogue boxes on my switch case method?

2 Answers2