0
\$\begingroup\$

I've done this assignment here for my android programming course I'm taking. The exercise requirements were to create a single-activity interface for selecting a 'photo package' to purchase, and to select the payment method, and then display a short summary of the information (subtotal, sales tax, total cost, and selected payment method), updating as things are selected or deselected on the screen.

Basically, there's a set of six checkboxes, and a set of radio buttons, and quite a few TextView items here in the Layout. I'm not asking about how I could have better laid out the items in the layout, but I'm including the layout file here anyways. And a preview picture of how it displays in a Nexus 5 screen size/layout preview. It and the code is below.

Again, I am not asking about improving the layout; it's here in case you need to refer to it.

Suffice it to say, this app works for the purposes of the assignment!


But, I'm a little concerned about how I could have better approached some of the logic in the code-level though. Namely, I am concerned with the processing and handling I have in place of the checkbox states.

I currently have a separate method/function here (setSubtotal) that takes six booleans (the isChecked() state of each checkbox on the layout), and then from those Boolean values determines the subtotal of the order. Sales tax and grand total are calculated afterwards, and then the TextViews for the order summary are updated. It does this by using six separate if statements to determine which checkboxes are checked, and I feel that's horridly inefficient.

Considering that this was homework that I've already completed and turned in, treat this as a learning exercise for me, and please don't crucify me for other failures I may have made (though I am open to suggestions on how I can improve). I'm more or less focused on processing which checkboxes are checked in a more efficient method, if there is one.

main_activity.xml:

(NOTE: @color/jet_black is in the colors.xml file, and is just there to refer to #000000 (black) color in the layout. I didn't feel it necessary to include an otherwise stock-default colors.xml file.)

<?xml version="1.0" encoding="utf-8"?>
<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    tools:context="teward.homework.schoolportraitsales.MainActivity"
    android:id="@+id/main_activity"
    android:layout_width="fill_parent"
    android:layout_height="match_parent"
    android:paddingBottom="@dimen/activity_vertical_margin"
    android:paddingLeft="@dimen/activity_horizontal_margin"
    android:paddingRight="@dimen/activity_horizontal_margin"
    android:paddingTop="@dimen/activity_vertical_margin"
    android:layout_alignParentTop="true"
    tools:ignore="RtlHardcoded,LabelFor">
    <!-- Note: IDE Warning Ignore Overrides in tools:ignore,
    and are comma separated.  Done to suppress IDE level warnings. -->

    <!-- Primary Package Selection -->
    <TextView
        android:id="@+id/txtPackageSelect"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:text="@string/standard_pkg_header"
        android:textColor="@color/jet_black"
        android:textSize="14sp"
        android:textStyle="italic"
        android:layout_alignParentTop="true"
        android:layout_centerHorizontal="true" />

    <GridLayout
        android:id="@+id/PackageSelectContainer"
        android:layout_below="@id/txtPackageSelect"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_centerHorizontal="true"
        android:layout_centerInParent="true"
        android:columnCount="1"
        android:rowCount="3">

        <CheckBox
            android:id="@+id/chkPackageA12"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/pkgA12"
            android:textSize = "12sp"
            android:textColor="@color/jet_black"
            android:buttonTint="@color/jet_black"
            android:layout_column="0"
            android:layout_row="0"/>

        <CheckBox
            android:id="@+id/chkPackageB12"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/pkgB12"
            android:textSize = "12sp"
            android:textColor="@color/jet_black"
            android:buttonTint="@color/jet_black"
            android:layout_column="0"
            android:layout_row="1" />

        <CheckBox
            android:id="@+id/chkPackageC12"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/pkgC12"
            android:textSize = "12sp"
            android:textColor="@color/jet_black"
            android:buttonTint="@color/jet_black"
            android:layout_column="0"
            android:layout_row="2" />

    </GridLayout>

    <!-- Extra Package Selection -->
    <TextView
        android:id="@+id/txtExtraPackageSelect"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_below="@id/PackageSelectContainer"
        android:layout_marginLeft="5dp"
        android:layout_marginTop="5dp"
        android:text="@string/extra_pkg_header"
        android:textColor="@color/jet_black"
        android:textSize="14sp"
        android:textStyle="italic"
        android:layout_centerHorizontal="true" />

    <GridLayout
        android:id="@+id/ExtraPackageSelectContainer"
        android:layout_below="@id/txtExtraPackageSelect"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_centerHorizontal="true"
        android:rowCount="3"
        android:columnCount="1">

        <CheckBox
            android:id="@+id/chkExtraPackage1"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/pkgExtra1"
            android:textSize = "12sp"
            android:textColor="@color/jet_black"
            android:buttonTint="@color/jet_black"
            android:layout_row="0" />

        <CheckBox
            android:id="@+id/chkExtraPackage2"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/pkgExtra2"
            android:textSize = "12sp"
            android:textColor="@color/jet_black"
            android:buttonTint="@color/jet_black"
            android:layout_row="1" />

        <CheckBox
            android:id="@+id/chkExtraPackage3"
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:text="@string/pkgExtra3"
            android:textSize = "12sp"
            android:textColor="@color/jet_black"
            android:buttonTint="@color/jet_black"
            android:layout_row="2" />

    </GridLayout>

    <!-- Payment Options -->
    <TextView
        android:id="@+id/txtPaymentOptions"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_below="@id/ExtraPackageSelectContainer"
        android:layout_marginLeft="5dp"
        android:layout_marginTop="5dp"
        android:layout_centerHorizontal="true"
        android:text="@string/payment_options_headers"
        android:textSize = "14sp"
        android:textColor="@color/jet_black"
        android:textStyle="italic" />

    <RadioGroup
        android:id="@+id/grpPaymentSelect"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_below="@id/txtPaymentOptions"
        android:layout_marginTop="5dp"
        android:layout_centerHorizontal="true">

        <RadioButton
            android:id="@+id/radioPaymentCredit"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/payment_credit"
            android:textSize = "12sp"
            android:checked="true"
            android:buttonTint="@color/jet_black" />

        <RadioButton
            android:id="@+id/radioPaymentCheck"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/payment_check"
            android:textSize = "12sp"
            android:buttonTint="@color/jet_black" />

        <RadioButton
            android:id="@+id/radioPaymentCash"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/payment_cash"
            android:textSize = "12sp"
            android:buttonTint="@color/jet_black" />

    </RadioGroup>

    <!-- Summary Section -->
    <GridLayout
        android:id="@+id/SummaryContainer"
        android:layout_below="@id/grpPaymentSelect"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_centerHorizontal="true"
        android:layout_marginTop="10dp"
        android:rowCount="5"
        android:columnCount="2">

        <TextView
            android:id="@+id/txtSummary1"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:text="@string/summary1"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"
            android:textStyle="bold|italic"
            android:gravity="center"

            android:layout_column="0"
            android:layout_columnSpan="2"
            android:layout_row="0" />

        <TextView
            android:id="@+id/txtSummary2"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/summary2"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"

            android:layout_column="0"
            android:layout_row="1" />

        <TextView
            android:id="@+id/txtSubtotal"
            android:layout_height="wrap_content"
            android:layout_width="80sp"
            android:text="@string/initial_cost_placeholder"
            android:textColor="@color/jet_black"
            android:textSize="14sp"
            android:gravity="right"

            android:layout_column="1"
            android:layout_row="1" />

        <TextView
            android:id="@+id/txtSummary3"
            android:layout_height="wrap_content"
            android:layout_width="wrap_content"
            android:text="@string/summary3"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"

            android:layout_column="0"
            android:layout_row="2" />

        <TextView
            android:id="@+id/txtSalesTax"
            android:layout_height="wrap_content"
            android:layout_width="80sp"
            android:text="@string/initial_cost_placeholder"
            android:textColor="@color/jet_black"
            android:textSize="14sp"
            android:gravity="right"

            android:layout_column="1"
            android:layout_row="2" />

        <TextView
            android:id="@+id/txtSummary4"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:text="@string/summary4"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"

            android:layout_column="0"
            android:layout_row="3" />

        <TextView
            android:id="@+id/txtTotal"
            android:layout_height="wrap_content"
            android:layout_width="80sp"
            android:text="@string/initial_cost_placeholder"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"
            android:gravity="right"

            android:layout_column="1"
            android:layout_row="3" />

        <TextView
            android:id="@+id/txtSummary5"
            android:layout_height="wrap_content"
            android:layout_width="wrap_content"
            android:text="@string/summary5"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"

            android:layout_column="0"
            android:layout_row="4" />

        <TextView
            android:id="@+id/txtPaymentMethod"
            android:layout_height="wrap_content"
            android:layout_width="80sp"
            android:text="@string/initial_payment_placeholder"
            android:textColor="@color/jet_black"
            android:textSize = "14sp"
            android:gravity="right"

            android:layout_column="1"
            android:layout_row="4" />

    </GridLayout>

</RelativeLayout>

Looks a bit like this:

Layout Picture

MainActivity.java:

package teward.homework.schoolportraitsales;

// This SuppressLint import is needed for IDE warnings suppression to work.
import android.annotation.SuppressLint;
import android.support.v7.app.AppCompatActivity;
import android.os.Bundle;
import android.widget.CheckBox;
import android.widget.CompoundButton;
import android.widget.RadioButton;
import android.widget.RadioGroup;
import android.widget.TextView;

public class MainActivity extends AppCompatActivity {

    private double orderSubtotal;
    private double orderTax;
    private double orderTotal;
    private String orderPaymentMethod;

    private CheckBox chkPackageA12;
    private CheckBox chkPackageB12;
    private CheckBox chkPackageC12;
    private CheckBox chkExtraPackage1;
    private CheckBox chkExtraPackage2;
    private CheckBox chkExtraPackage3;

    private TextView txtSubtotal;
    private TextView txtSalesTax;
    private TextView txtTotal;
    private TextView txtPaymentMethod;

    // IDE Warning Suppression
    @SuppressWarnings("FieldCanBeLocal")
    private RadioGroup paymentSelect;
    private RadioButton pmtCredit;
    private RadioButton pmtCheck;
    private RadioButton pmtCash;


    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main_activity);

        orderSubtotal = 0.00;
        orderTax = 0.00;
        orderTotal = 0.00;

        // Default payment method will be Credit Card, and that button will be autoselected.
        orderPaymentMethod = "Credit Card";

        //Match variables in Layout to Program variables

        //Primary Packages
        chkPackageA12 = (CheckBox) findViewById(R.id.chkPackageA12);
        chkPackageA12.setOnCheckedChangeListener(packageChangeListener);
        chkPackageB12 = (CheckBox) findViewById(R.id.chkPackageB12);
        chkPackageB12.setOnCheckedChangeListener(packageChangeListener);
        chkPackageC12 = (CheckBox) findViewById(R.id.chkPackageC12);
        chkPackageC12.setOnCheckedChangeListener(packageChangeListener);

        //Extra Packages
        chkExtraPackage1 = (CheckBox) findViewById(R.id.chkExtraPackage1);
        chkExtraPackage1.setOnCheckedChangeListener(packageChangeListener);
        chkExtraPackage2 = (CheckBox) findViewById(R.id.chkExtraPackage2);
        chkExtraPackage2.setOnCheckedChangeListener(packageChangeListener);
        chkExtraPackage3 = (CheckBox) findViewById(R.id.chkExtraPackage3);
        chkExtraPackage3.setOnCheckedChangeListener(packageChangeListener);

        //Payment Method Group and Buttons
        paymentSelect = (RadioGroup) findViewById(R.id.grpPaymentSelect);
        paymentSelect.setOnCheckedChangeListener(paymentChangeListener);
        pmtCredit = (RadioButton) findViewById(R.id.radioPaymentCredit);
        pmtCheck = (RadioButton) findViewById(R.id.radioPaymentCheck);
        pmtCash = (RadioButton) findViewById(R.id.radioPaymentCash);

        txtSubtotal = (TextView) findViewById(R.id.txtSubtotal);
        txtSalesTax = (TextView) findViewById(R.id.txtSalesTax);
        txtTotal = (TextView) findViewById(R.id.txtTotal);
        txtPaymentMethod = (TextView) findViewById(R.id.txtPaymentMethod);

    }

    private void setSubtotal(boolean A12, boolean B12, boolean C12,
                               boolean Extra1, boolean Extra2, boolean Extra3) {
        // Method designed to calculate Subtotal based on checkbox selections
        orderSubtotal = 0.00; // Necessary reinitialization.
        if (A12) {
            orderSubtotal += 47.50;
        }
        if (B12) {
            orderSubtotal += 44.50;
        }
        if (C12) {
            orderSubtotal += 39.50;
        }
        if (Extra1) {
            orderSubtotal += 11.00;
        }
        if (Extra2) {
            orderSubtotal += 15.00;
        }
        if (Extra3) {
            orderSubtotal += 11.00;
        }
    }

    // IDE Warning Suppression
    @SuppressLint("DefaultLocale")
    private double getSalesTax(double subtotal) {
        // Method designed to return the amount of sales tax on a subtotal.
        double taxRate = 0.06;  // 6% Sales Tax in PA
        return subtotal * taxRate;
    }

    //The CheckBox.OnCheckedChangeListener is actually just CompoundButton.OnCheckedChangeListener,
    //so we will create a single OnCheckedChangeListener to listen to all package changes, and
    //ignore the built in arguments (and grab them ourselves).
    private CompoundButton.OnCheckedChangeListener packageChangeListener =
            new CompoundButton.OnCheckedChangeListener() {

        // IDE Warning Suppression Needed Here
        @SuppressLint("DefaultLocale")
        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {

            setSubtotal(chkPackageA12.isChecked(), chkPackageB12.isChecked(),
                        chkPackageC12.isChecked(), chkExtraPackage1.isChecked(),
                        chkExtraPackage2.isChecked(), chkExtraPackage3.isChecked());
            orderTax = getSalesTax(orderSubtotal);
            orderTotal = orderSubtotal + orderTax;

            txtSubtotal.setText(String.format("$%.2f", orderSubtotal));
            txtSalesTax.setText(String.format("$%.2f", orderTax));
            txtTotal.setText(String.format("$%.2f", orderTotal));
        }
    };

    // We need to use a different OnCheckedChangeListener here for the RadioGroup, because it has
    // a little bit of evil happen when we try and use the CompoundButton.OnCheckedChangeListener.
    private RadioGroup.OnCheckedChangeListener paymentChangeListener =
            new RadioGroup.OnCheckedChangeListener() {

        // IDE Warnings Override
        @SuppressLint("DefaultLocale")
        @Override
        public void onCheckedChanged(RadioGroup rbGroup, int radioId) {

            if (pmtCredit.isChecked()){
                orderPaymentMethod = "Credit Card";
            } else if (pmtCheck.isChecked()){
                orderPaymentMethod = "Check";
            } else if (pmtCash.isChecked()) {
                orderPaymentMethod = "Cash";
            }

            txtPaymentMethod.setText(orderPaymentMethod);
        }
    };
}
\$\endgroup\$
2
\$\begingroup\$

In the CompoundButton.OnCheckedChangeListener, you don't have to get the check status of every single checkbox. The buttonView parameter in onCheckChanged will tell you which checkbox has changed, and you can add or subtract the subtotal based on whether the checkbox is checked or unchecked using the isChecked boolean.

Here's a sample solution. Take note of how the problem is broken down:

public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
    double amount = 0.0;

    /*
     * Here is where you check which checkbox has changed by checking its
     * id. Some people prefer to check the entire object:
     *
     * if (buttonView == chkExtraPackage1) {
     *     ...Insert rubbish here...
     * }
     *
     * But you can't use switch statements with that :P
     */
    switch (buttonView.getId()) {
        case R.id.chkExtraPackage1:
            amount = 11.0;
            break;
        case R.id.chkExtraPackage2:
            amount = 15.0;
            break;
        // Add the remaining checkboxes here
        default:
            break;
    }

    /* Here is where you add or subtract based on the new checked status */
    if (isChecked) {
        orderSubTotal += amount;
    } else {
        orderSubTotal -= amount;
    }
}

In fact, you can adapt this solution to shorten your RadioGroup.OnCheckedChangeListener since you're simply going to display the radio button's text:

/* 
 * radioId, according to the documentation, is the id of the RadioButton
 * in the RadioGroup that has been selected
 */
public void onCheckedChanged(RadioGroup rbGroup, int radioId) {
    RadioButton selectedRb = (RadioGroup) findViewById(radioId);
    txtPaymentMethod.setText(selectedRb.getText());
}

And using this means that you don't need all those findViewById statements for your radio buttons.


Edit: Here's a fun challenge. Shorten this code further with a HashMap.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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