Android - register button click and take action based on radio selection

Question

I'm trying to teach myself how to write android apps and I'm having trouble registering a button click and taking actions based on which radio button is selected at the time. This is a simple tip calculator:

import android.app.Activity;
import android.os.Bundle;
import android.widget.Button;
import android.widget.EditText;
import android.widget.RadioButton;
import android.widget.TextView;
import android.widget.RadioGroup;
import android.view.View;

public class TipCalc extends Activity implements RadioGroup.OnCheckedChangeListener,View.OnClickListener
{
    TextView result;
    RadioGroup radiogroup1;
    RadioButton r1,r2,r3;
    Button calculate;
    EditText bill, resulttotal;
    private int radioCheckedId = -1;

    @Override
    protected void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        radiogroup1 = (RadioGroup) findViewById(R.id.radiogroup1);
        Button calculate = (Button) findViewById(R.id.calculate); 
        RadioButton r1 = (RadioButton) findViewById(R.id.poor);
        RadioButton r2 = (RadioButton) findViewById(R.id.average);
        RadioButton r3 = (RadioButton) findViewById(R.id.excellent);
        EditText bill = new EditText(this);
        EditText resulttotal = new EditText(this);
        radiogroup1.setOnCheckedChangeListener(this);
        calculate.setOnClickListener(this); 
        //bill.setText("0");
        //resulttotal.setText("0");
     }

    public void onCheckedChanged(RadioGroup group, int checkedId) {
        radioCheckedId = checkedId;
    }

    public void onClick(View v)
        {
            if (v == calculate)
           {
                String billtotal;
                double total = 0;
                billtotal = bill.getText().toString();
                final int aInt = Integer.parseInt(billtotal);
                if (radioCheckedId == 1)
                {
                    total = aInt * 1.1;
                    final String aString = Double.toString(total);
                    resulttotal.setText(aString);
                }
               if (radioCheckedId == 2)
                {
                    total = aInt * 1.15;
                    final String aString = Double.toString(total);
                    resulttotal.setText(aString);
                }
               if (radioCheckedId == 3)
               {
                    total = aInt * 1.2;
                    final String aString = Double.toString(total);
                    resulttotal.setText(aString);
               }
            }
        }
}

Everything loads just fine, but nothing happens when I press the calculate button in the virtual phone.

Answer 1

The problem is where you're comparing the RadioGroup's selected id... you'll want to change your onClick() to:

public void onClick(View v) {
    if (v == calculate) {
        String billtotal;
        double total = 0;
        billtotal = bill.getText().toString();
        final int aInt = Integer.parseInt(billtotal);
        if (radioCheckedId == R.id.poor) {
            total = aInt * 1.1;
            final String aString = Double.toString(total);
            resulttotal.setText(aString);
        }
        if (radioCheckedId == R.id.average) {
            total = aInt * 1.15;
            final String aString = Double.toString(total);
            resulttotal.setText(aString);
        }
        if (radioCheckedId == R.id.excellent) {
            total = aInt * 1.2;
            final String aString = Double.toString(total);
            resulttotal.setText(aString);
        }
    }
}    

onCheckedChanged() gives you will be the R.id for the view and not just a number which tells you which it is in sequence.

A few quick (unrelated) suggestions:

• Use a switch statement instead of a bunch of if-statements.
• Put something in there to check for -1 (nothing checked) too... just to be sure.
• In the onClick() I usually check for which View was clicked by checking the incoming view's id. This just makes it where you don't have to keep everything stored and (IMHO) is a little more clear what you're talking about.

The above suggestions would look something like:

public void onClick(View v) {
    if (v.getId() == R.id.calculate) {
        String billtotal;
        double total = 0;
        billtotal = bill.getText().toString();
        final int aInt = Integer.parseInt(billtotal);
        switch(radioCheckedId) {
            case R.id.poor:
                total = aInt * 1.1;
                final String aString = Double.toString(total);
                resulttotal.setText(aString);
                break;
            case R.id.average:
                total = aInt * 1.15;
                final String aString = Double.toString(total);
                resulttotal.setText(aString);
                break;
            case R.id.excellent:
                total = aInt * 1.2;
                final String aString = Double.toString(total);
                resulttotal.setText(aString);
                break;
            default:
                // do something for when nothing is selected... maybe throw an error?
                break;
        }
    }
}

Lastly, if all you're doing in onCheckedChanged() is storing the value you could get rid of it all together and just check for it in the onClick(). Something like:

public void onClick(View v) {
    int radioCheckedId = radiogroup1.getCheckedRadioButtonId();
    if (v == calculate) {
        // ...

Unrelated, but another problem I noticed (and someone else mentioned)... if your EditTexts are listed in the XML layout then you'd need to get hooks to them like this (and not create new ones):

EditText bill        = (EditText) findViewById(R.id.bill       );
EditText resulttotal = (EditText) findViewById(R.id.resulttotal);

Also, you could probably just use a TextView instead of an EditView for the result if yo udon't need it to be editable.

Answer 2

import java.text.NumberFormat;
import java.util.Locale;
import android.app.Activity;
import android.os.Bundle;
import android.widget.Button;
import android.widget.EditText;
import android.widget.RadioButton;
import android.widget.TextView;
import android.widget.RadioGroup;
import android.view.View;

public class TipCalc extends Activity
{
    TextView result;
    RadioGroup radiogroup1;
    RadioButton r1,r2,r3;
    Button calculate;
    EditText bill, resulttotal;
    Locale currentLocale = Locale.getDefault();

    @Override
    protected void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        radiogroup1 = (RadioGroup) findViewById(R.id.radiogroup1);
        final Button calculate = (Button) findViewById(R.id.calculate); 
        final RadioButton r1 = (RadioButton) findViewById(R.id.poor);
        final RadioButton r2 = (RadioButton) findViewById(R.id.average);
        final RadioButton r3 = (RadioButton) findViewById(R.id.excellent);
        final EditText bill = (EditText) findViewById(R.id.bill);
        final EditText tiptotal = (EditText) findViewById(R.id.tiptotal);
        final EditText resulttotal = (EditText) findViewById(R.id.resulttotal);
        bill.setText("0.00");
        tiptotal.setText("0.00");
        resulttotal.setText("0.00");
        calculate.setOnClickListener(new View.OnClickListener() {
            public void onClick(View v) throws  NumberFormatException {
                if (v == calculate)
                {
                NumberFormat currencyFormatter;
                currencyFormatter = NumberFormat.getCurrencyInstance(currentLocale);
                double atotal = 0;
                    double btotal = 0;
                    String billtotal = bill.getText().toString();
                    Double aDbl = 0.00;
                    try
                    {
                        aDbl = Double.parseDouble(billtotal);
                    }
                    catch(NumberFormatException n)
                    {
                        aDbl = 0.00;
                    }
                    if (r1.isChecked())
                     {
                        atotal = aDbl * 1.1;
                        btotal = aDbl * 0.1;
                     }
                    if (r2.isChecked())
                     {
                        atotal = aDbl * 1.15;
                        btotal = aDbl * 0.15;
                    }
                    if (r3.isChecked())
                    {
                        atotal = aDbl * 1.2;
                        btotal = aDbl * 0.2;
                    }
                    final String bString = currencyFormatter.format(btotal);
                    tiptotal.setText(bString);
                    final String aString = currencyFormatter.format(atotal);
                    resulttotal.setText(aString);
                 }
            }
        });

     }
}