9
\$\begingroup\$

After learning a bit of Android Java I've chosen to make a tiny game so that I can use the learned techniques in a real app.

I had the idea of a slot machine simulator:

You press a button and it generates three random numbers. The sum of the random numbers becomes your final result and a sound is played. Depending on the result a message is displayed.

Before starting a game.

After starting a game.

MainActivity.java:

package com.example.michael.slotmachine;

import android.media.MediaPlayer;
import android.support.v7.app.AppCompatActivity;
import android.os.Bundle;
import android.view.View;
import android.widget.TextView;
import android.widget.Toast;
import java.util.Random;

public class MainActivity extends AppCompatActivity {
    TextView tvSum;
    TextView tvRoll1;
    TextView tvRoll2;
    TextView tvRoll3;
    Random random;
    MediaPlayer mp3Player;

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

        // Init of object-props goes here ...
        tvSum       = (TextView) findViewById(R.id.textViewSum);
        tvRoll1     = (TextView) findViewById(R.id.textViewRoll1);
        tvRoll2     = (TextView) findViewById(R.id.textViewRoll2);
        tvRoll3     = (TextView) findViewById(R.id.textViewRoll3);
        random      = new Random();
        mp3Player   = MediaPlayer.create(this, R.raw.beep);
        // -------------------------------------------------------
    }

    public void processGame(View v) {
        // Do the math ...
        int roll1 = 1 + random.nextInt(7);
        int roll2 = 1 + random.nextInt(7);
        int roll3 = 1 + random.nextInt(7);

        int rollsSum = roll1 + roll2 + roll3;

        // ... then write the results to the UI.
        tvRoll1.setText("" + roll1);
        tvRoll2.setText("" + roll2);
        tvRoll3.setText("" + roll3);

        tvSum.setText("" + rollsSum);

        v.setEnabled(false);    // Disable the 'Spin Roll'-Button.

        mp3Player.start();      // Play the MP3-file.

        // ---- Pop-Up Message -------
        String message;

        if (rollsSum < 10) {
            message = "Better luck next time.";
        } else if (rollsSum < 15) {
            message = "Respectable result.";
        } else if (rollsSum < 21) {
            message = "You have done great.";
        } else {
            message = "You have hit the Jackpot.";
        }

        Toast toast  = Toast.makeText(
                                getApplicationContext(),
                                message,
                                Toast.LENGTH_SHORT);
        toast.show();
        // ------------------------------
    }

    public void recreateGame(View v) {
        finish();
        startActivity(getIntent());
    }
}

activity_main.xml:

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:orientation="vertical"
    android:weightSum="100">

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="0px"
        android:layout_weight="20"
        android:background="@android:color/holo_blue_light"
        android:orientation="horizontal"
        android:weightSum="1">

        <TextView
            android:id="@+id/textViewRoll1"
            android:layout_width="106dp"
            android:layout_height="40dp"
            android:layout_marginLeft="30px"
            android:layout_marginRight="10dp"
            android:layout_marginTop="40px"
            android:layout_weight="0.24"
            android:textAlignment="center"
            android:textSize="24sp" />

        <TextView
            android:id="@+id/textViewRoll2"
            android:layout_width="106dp"
            android:layout_height="40dp"
            android:layout_marginLeft="10px"
            android:layout_marginRight="10dp"
            android:layout_marginTop="40px"
            android:textAlignment="center"
            android:textSize="24sp" />

        <TextView
            android:id="@+id/textViewRoll3"
            android:layout_width="106dp"
            android:layout_height="40dp"
            android:layout_marginLeft="10px"
            android:layout_marginRight="10dp"
            android:layout_marginTop="40px"
            android:textAlignment="center"
            android:textSize="24sp" />
    </LinearLayout>

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="0px"
        android:orientation="vertical"
        android:layout_weight="40"
        android:background="@android:color/holo_orange_light"
        android:weightSum="1">

        <TextView
            android:id="@+id/textViewYourScore"
            android:layout_width="match_parent"
            android:layout_height="40dp"
            android:layout_marginLeft="20px"
            android:layout_marginRight="20dp"
            android:layout_marginTop="40px"
            android:layout_weight="0.11"
            android:text="Your Score:"
            android:textAlignment="center"
            android:textSize="30sp" />

        <TextView
            android:id="@+id/textViewSum"
            android:layout_width="match_parent"
            android:layout_height="40dp"
            android:layout_marginLeft="20px"
            android:layout_marginRight="20dp"
            android:layout_marginTop="40px"
            android:layout_weight="0.16"
            android:textAlignment="center"
            android:textSize="30sp" />

    </LinearLayout>

    <LinearLayout
        android:layout_width="match_parent"
        android:layout_height="0px"
        android:layout_weight="40"
        android:background="@android:color/holo_red_light"
        android:orientation="vertical">

        <Button
            android:id="@+id/buttonSpin"
            android:layout_width="match_parent"
            android:layout_height="60dp"
            android:onClick="processGame"
            android:text="Spin Roll"
            android:textSize="30sp" />

        <Button
            android:id="@+id/buttonNewGame"
            android:layout_width="match_parent"
            android:layout_height="60dp"
            android:layout_marginTop="50px"
            android:onClick="recreateGame"
            android:text="New Game"
            android:textSize="30sp" />
    </LinearLayout>

</LinearLayout>

Is my code done in a good fashion? Are there points which I should improve?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Not really a code problem, but your logic is not how a slot machine works. You don't sum the numbers, you win if they match certain patterns, e.g. three sevens. \$\endgroup\$ – jamesqf Jul 30 '17 at 17:34
10
\$\begingroup\$

Clear state rather than restart

You use this function to restart the game:

public void recreateGame(View v) {
    finish();
    startActivity(getIntent());
}

This recreates the activity. I don't think this is a recommended practice. I think it would be better to restore the initial state of the view elements. The same method that does that could also be used for the initial setup.

Initializing Random

It's not a big deal, but since the instance of Random doesn't depend on the call to onCreate, I would initialize it at the time of declaration. That way I surely won't forget to initialize it later.

Unnecessary local variable

The local toast variable is unnecessary here, you could drop it and call .show() directly chained after .makeText(...).

Toast.makeText(
    getApplicationContext(),
    message,
    Toast.LENGTH_SHORT).show();
\$\endgroup\$
6
\$\begingroup\$
        // Do the math ...

Doesn't provide much explanation. I would instead say:

        // Roll 3 dice and collect their sum.

To illustrate what the random numbers are doing.

Same goes for:

        mp3Player.start();      // Play the MP3-file.

What MP3-file? What does this have to do with the slot machine?

This comment:

       v.setEnabled(false);    // Disable the 'Spin Roll'-Button.

is pretty good, it tells me what this line of code does in terms of the role it plays in the slot machine.

\$\endgroup\$
4
\$\begingroup\$

Overall it seems clear enough. I'd like to quibble with a pair of method names, these callback handlers:

public void processGame(View v) ...

and

public void recreateGame(View v) ...

It's good advice to strive for "boring" in many aspects of software engineering, especially in android development. The identifiers buttonSpin and buttonNewGame are terrific. They're clear, concise, and they match the on-screen appearance, so they're easy to find. Reuse such names in related parts of an app. I'd recommend naming your handlers onSpin and onNewGame.

I agree with Janos about sticking to recommended practice. See also https://developer.android.com/guide/components/activities/activity-lifecycle.html#alc

\$\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.