0
\$\begingroup\$

I'm learning to code Android apps and have recently finished a quiz app. I would like some feedback about best practices and possible ways to improve performance, as I'm also new to Java.

activity_main.xml:

<?xml version="1.0" encoding="utf-8"?>
<ScrollView xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:fillViewport="true"
    tools:context="com.example.android.mathquiz.MainActivity"
    android:background="#C5CAE9">

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:focusable="true"
        android:focusableInTouchMode="true"
        android:gravity="center_horizontal"
        android:orientation="vertical"
        android:paddingBottom="16dp"
        android:paddingLeft="16dp"
        android:paddingRight="16dp">

        <TextView
            style="@style/AllElements"
            android:text="Respondé las siguientes preguntas:" />

        <!-- QUESTION 1 [DONE]-->
        <TextView
            style="@style/Questions"
            android:text="1) La pendiente de una recta es su inclinación." />

        <RadioGroup
            style="@style/AllElements"
            android:id="@+id/question_one_answers"
            android:layout_width="wrap_content"
            android:layout_height="wrap_content">

            <RadioButton
                android:id="@+id/question_one_answer_one"
                android:layout_width="wrap_content"
                android:layout_height="wrap_content"
                android:text="Verdadero"/>

            <RadioButton
                android:id="@+id/question_one_answer_two"
                android:layout_width="wrap_content"
                android:layout_height="wrap_content"
                android:text="Falso"/>
        </RadioGroup>

    <!-- QUESTION 2 [LISTO]-->
    <TextView
        style="@style/Questions"
        android:text="2) Indicar la ordenada al origen de la función:\ny = x" />

    <EditText
        style="@style/AllElements"
        android:id="@+id/question_two_answer"
        android:layout_width="match_parent"
        android:inputType="number"
        android:hint="Respuesta"/>

    <!-- QUESTION 3 [LISTO]-->
    <TextView
        style="@style/Questions"
        android:text="3) Indicar la pendiente de la función:\ny = x + 1" />

    <EditText
        style="@style/AllElements"
        android:id="@+id/question_three_answer"
        android:layout_width="match_parent"
        android:inputType="number"
        android:hint="Respuesta"/>

    <!-- QUESTION 4 -->
    <TextView
        style="@style/Questions"
        android:layout_marginBottom="0dp"
        android:text="4) ¿Cuáles son los requisitos de las funciones?" />

    <TextView
        style="@style/AllElements"
        android:layout_marginBottom="32dp"
        android:text="Seleccionar todas las opciones posibles." />

    <CheckBox
        android:id="@+id/question_four_answer_one"
        style="@style/AllElements"
        android:text="Unicidad" />

    <CheckBox
        android:id="@+id/question_four_answer_two"
        style="@style/AllElements"
        android:text="Positividad" />

    <CheckBox
        android:id="@+id/question_four_answer_three"
        style="@style/AllElements"
        android:text="Existencia" />

    <!-- CHECK ANSWERS -->
    <Button
        style="@style/AllElements"
        android:text="Revisar"
        android:layout_marginBottom="8dp"
        android:layout_marginTop="36dp"
        android:onClick="checkAnswers"/>


</LinearLayout>

</ScrollView>

MainActivity.java:

public class MainActivity extends AppCompatActivity {
    String[] correctAnswers = {"Falso", "0", "1", "Unicidad Existencia"};

    RadioGroup questionOneAnswers;

    RadioButton questionOneAnswerOne;
    RadioButton questionOneAnswerTwo;
    EditText questionTwoAnswer;
    EditText questionThreeAnswer;
    CheckBox questionFourAnswerOne;
    CheckBox questionFourAnswerTwo;
    CheckBox questionFourAnswerThree;

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

        // QUESTION 1
        questionOneAnswerOne = (RadioButton) findViewById(R.id.question_one_answer_one);
        questionOneAnswerTwo = (RadioButton) findViewById(R.id.question_one_answer_two);

        // QUESTION 2
        questionTwoAnswer = (EditText) findViewById(R.id.question_two_answer);

        // QUESTION 3
        questionThreeAnswer = (EditText) findViewById(R.id.question_three_answer);

        // QUESTION 4
        questionFourAnswerOne = (CheckBox) findViewById(R.id.question_four_answer_one);
        questionFourAnswerTwo = (CheckBox) findViewById(R.id.question_four_answer_two);
        questionFourAnswerThree = (CheckBox) findViewById(R.id.question_four_answer_three);

        questionOneAnswers = (RadioGroup) findViewById(R.id.question_one_answers);

    }

    private boolean isEmpty(EditText myeditText) {
        return myeditText.getText().toString().trim().length() == 0;
    }

    private String questionOneGetAnswer() {
        if (questionOneAnswerOne.isChecked()) {
            return questionOneAnswerOne.getText().toString();
        }

        else if (questionOneAnswerTwo.isChecked()) {
            return questionOneAnswerTwo.getText().toString();
        }

        else {
            return "";
        }
    }

    private String questionTwoGetAnswer() {
        if (!isEmpty(questionTwoAnswer)) {
            return questionTwoAnswer.getText().toString();
        }

        else {
            return "";
        }
    }

    private String questionThreeGetAnswer() {
        if (!isEmpty(questionThreeAnswer)) {
            return questionThreeAnswer.getText().toString();
        }

        else {
            return "";
        }
    }

    private String questionFourGetAnswer () {
        String returnString = "";

        if (questionFourAnswerOne.isChecked()) {
            returnString += questionFourAnswerOne.getText();
        }

        if (questionFourAnswerTwo.isChecked()) {
            returnString += " " + questionFourAnswerTwo.getText();
        }

        if (questionFourAnswerThree.isChecked()) {
            returnString += " " + questionFourAnswerThree.getText();
        }

        return returnString;
    }

    public void checkAnswers(View view) {
        String[] givenAnswers = {questionOneGetAnswer(), questionTwoGetAnswer(), questionThreeGetAnswer(), questionFourGetAnswer()};

        int right = 0;
        int wrong = 0;

        for (int i = 0; i < givenAnswers.length; i++) {
            if (givenAnswers[i].equals(correctAnswers[i])) {
                right++;
            }

            else {
                wrong++;
            }
        }

        String message;

        if (wrong == 0) {
            message = "You got all the questions right!";
        }

        else {
            message = "Respuestas correctas: " + right + "\nRespuestas incorrectas: " + wrong + "\n¡Inténtalo de nuevo!";
        }

        //Reset answer fields
        // QUESTION 1
        questionOneAnswers.clearCheck();

        // QUESTION 2
        questionTwoAnswer.setText("");

        // QUESTION 3
        questionThreeAnswer.setText("");

        // QUESTION 4
        questionFourAnswerOne.setChecked(false);
        questionFourAnswerTwo.setChecked(false);
        questionFourAnswerThree.setChecked(false);

        // Print out message with quiz results
        Toast.makeText(this, message, Toast.LENGTH_SHORT).show();
    }
}

styles.xml:

<resources>

<!-- Base application theme. -->
<style name="AppTheme" parent="Theme.AppCompat.Light.DarkActionBar">
    <!-- Customize your theme here. -->
    <item name="colorPrimary">@color/colorPrimary</item>
    <item name="colorPrimaryDark">@color/colorPrimaryDark</item>
    <item name="colorAccent">@color/colorAccent</item>
    <item name="android:textColor">@color/colorAccent</item>
</style>

<style name="AllElements">
    <item name="android:layout_width">wrap_content</item>
    <item name="android:layout_height">wrap_content</item>
    <item name="android:layout_marginTop">16dp</item>
    <item name="android:textSize">18sp</item>
    <item name="android:textAlignment">center</item>
    <item name="android:textColor">#0D47A1</item>
</style>

<style name="Questions" parent="AllElements">
    <item name="android:layout_marginBottom">16dp</item>
    <item name="android:layout_marginTop">32dp</item>
    <item name="android:textSize">26sp</item>
</style>

</resources>
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

1) You can remove the below function and directly use TextUtils.isEmpty

2) questionThreeGetAnswer() & questionTwoGetAnswer() can be refactor

 private String questionThreeGetAnswer() {
        return questionThreeAnswer.getText().toString();
 }

as questionThreeAnswer.getText().toString() always return "", if there is no data. So need to check for empty & return "".

Note:

In your case, if-else statement is useful only if you have to show an error for an empty answer.

\$\endgroup\$
1
  • \$\begingroup\$ Welcome :) glad to help you \$\endgroup\$
    – N J
    Commented May 24, 2017 at 14:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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