2
\$\begingroup\$

Alright, so I've been tasked with recreating the game of pig dice in Java, however the issue that im having with creating a method that switches back and fourth between pc and user is that there's a lot of redundancy with calling methods with different parameters.

How would I be able to condense this alternating code?

if (coin.equals("heads")) {
    if (currentPlayer.equals(pcName)) {
        pcPoints = roll2Dices(kb, random, currentPlayer, pcPoints, userName, pcName, round);
        currentPlayer = userName;
    } else if (currentPlayer.equals(userName)) {
        userPoints = roll2Dices(kb, random, currentPlayer, userPoints, userName, pcName, round);
        currentPlayer = pcName;
    }
} else if (coin.equals("tails")) {
    if (currentPlayer.equals(userName)) {
        userPoints = roll2Dices(kb, random, currentPlayer, userPoints, userName, pcName, round);
        currentPlayer = pcName;
    } else if (currentPlayer.equals(pcName)) {
        pcPoints = roll2Dices(kb, random, currentPlayer, pcPoints, userName, pcName, round);
        currentPlayer = userName;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Why check for "heads" or "tails" if the following code does the same in both cases? \$\endgroup\$ – Marvin Nov 5 '17 at 18:54
  • \$\begingroup\$ Oh man. I just realized this... Thank YOU! It looks so much cleaner now. \$\endgroup\$ – NoName Nov 5 '17 at 21:25
1
\$\begingroup\$

Rethink your function

Instead of using if statements you can store the points in an array and then the current player variable could be an index for the array (so not a string value, use and integer instead).

Try to think in numbers instead of strings.

Code example:

int[2] pointsArray;
int currentPlayer = 0;
pointsArray[currentPlayer] = roolDiceFunction();

If you really want to use strings though you could use a map but I think it will just make your code more complicated.

Coin:

Just do the coin check at the start of your function by setting the start index for current player. For example if you want the user to start if the coin lands on heads which you can represent as 0, you then just set the current player index to a random value between 0 & 1.

Example:

int[2] pointsArray;
int currentPlayer = (int)Math.random(); //Random value between 0 & 1
pointsArray[currentPlayer] = roolDiceFunction();

I'm guessing there's more to your function than this, but you probably want to have it in a loop of some kind and then ask at the end of the function if the user wants to keep rolling, if they don't want to keep rolling you can change the current player index.

More about your functions

Make the rool2Dice function simpler, does it really need that many parameters, you are passing the exact same most of the time.

If you have any more questions just ask away, good luck.

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