views:

3273

answers:

3

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.

A: 

Your problem is that you never add EditText instances to the current layout.

You should add them as children of the main layout.

Lucas S.
you mean changing it to this?EditText bill = (EditText) findViewById(R.id.bill);EditText resulttotal = (EditText) findViewById(R.id.resulttotal);
MaQleod
That is A problem, but it is **NOT** THE problem.
fiXedd
what detriments are caused by adding them to the current layout instead of as children?
MaQleod
+2  A: 

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.

fiXedd
decided not to go with a switch statement as there are only 3 options and it doesn't seem necessary, although it would be cleaner. I check a radio button in the xml document, so there really should be no way to not have one checked, but I suppose better safe than sorry.The solution I found (posted below) had to do with how I handled the onclicklistener.
MaQleod
Yeah, the switch is kind of a personal preference thing. Looks like your solution should work too.
fiXedd
A: 
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);
                 }
            }
        });

     }
}
MaQleod