3
\$\begingroup\$

I am writing a computer opponent in java to play the dice game pig. I am trying to code it to be optimized based off of this paper.

    @Override
public boolean decideIfShouldRoll(int turnNumber, int rollNumber,
        int poolSize, int myScore, int opponentsScore) {
    if (myScore < 30 && opponentsScore < 30) {
        if (poolSize <= 20)
            return true;
        else
            return false;
    }

    // Checks for if the opponent has a greater score
    if (opponentsScore > myScore) {

        // opponent is near winning?
        if (100 - opponentsScore <= 20) {

            // checks if computer is near winning
            if (100 - myScore <= 20) {
                if (poolSize <= 20)
                    return true;
                else
                    return false;
            }

            // if computer not near winning, take a go for broke strategy
            if (100 - myScore > 20) {
                if (poolSize <= 30)
                    return true;
                else
                    return false;
            }

        }
        if (opponentsScore > 20) {
            if (poolSize <= 20)
                return true;
            else
                return false;
        }

        return true;
    }

    if (myScore > opponentsScore) {

        // if near winning and the opponent's score is within 10 try a small
        // wins strategy
        if ((100 - myScore) <= 20 && (myScore - opponentsScore) <= 10) {
            if (poolSize <= 15)
                return true;
            else
                return false;
        }
        // if 10 points from winning, only build a pool up to 10 points
        if ((100 - myScore) <= 10) {
            if (poolSize <= 10)
                return true;
            else
                return false;
        }
        // if the difference between the computer score and opponent score
        // is > 10, go up to the standard optimal pool size of 20
        if ((myScore - opponentsScore) > 10) {
            if (poolSize <= 20)
                return true;
            else
                return false;
        }

        // if near winning and the opponents score is within 10 points, take
        // a reduced standard pool size
        if ((100 - myScore) <= 20
                && Math.abs(myScore - opponentsScore) <= 10) {

            if (poolSize <= 15)
                return true;
            else
                return false;
        }

        return true;
    }
    return true;

}
\$\endgroup\$

migrated from stackoverflow.com Feb 13 '17 at 7:27

This question came from our site for professional and enthusiast programmers.

  • 2
    \$\begingroup\$ Does this code work at the moment? This is important; only working code can be posted for review on that site. \$\endgroup\$ – Martijn Pieters Feb 13 '17 at 7:21
  • \$\begingroup\$ Well i think this should not had been moved, since you kind of ask how to implement the algorithm presented in the linked paper and the code does not cover this algortihm, so it's not about optimizing but rewriting a whole function. \$\endgroup\$ – SchreiberLex Feb 13 '17 at 8:56
  • \$\begingroup\$ @Lex Even though the function might not implement the exact algorithm in the paper, would you say that it works in the current state? \$\endgroup\$ – Simon Forsberg Feb 13 '17 at 9:13
  • \$\begingroup\$ It does work. Well the decision to keep on rolling if both partys are same score and equal or bigger 30 until win or fail might not be best decision but thats maybe an artifical gamblers mind. Or do i miss another point? \$\endgroup\$ – SchreiberLex Feb 13 '17 at 9:32
1
\$\begingroup\$

Bugs

The outline of the function is

@Override
public boolean decideIfShouldRoll(int turnNumber, int rollNumber,
        int poolSize, int myScore, int opponentsScore) {
    if (myScore < 30 && opponentsScore < 30) {
        …
    }
    if (opponentsScore > myScore) {
        …
    }
    if (myScore > opponentsScore) {
        …
    }
    return true;
}

The return true at the end is problematic. You're saying that, except in the opening moves, anytime the score is tied, you want to roll forever, essentially forfeiting your turn.


If we have 95 points and a pool of 5, we should stop rolling. Your code tries to amass a pool of 15 points or 10 points.

Redundancy

The opponentScore > 20 test should always be true, since the opponent is in the lead, and the leader has at least 30 points.

@Override
public boolean decideIfShouldRoll(int turnNumber, int rollNumber,
        int poolSize, int myScore, int opponentsScore) {
    if (myScore < 30 && opponentsScore < 30) {
        …
    }
    if (opponentsScore > myScore) {
        …
        if (opponentsScore > 20) {
            …
        }
        return false;
    }
    if (myScore > opponentsScore) {
        …
    }
    return true;
}

Math.abs() is superfluous here, since we have already established that we are in the lead:

if (myScore > opponentsScore) {
    if ((100 - myScore) <= 20 && (myScore - opponentsScore) <= 10) {
        …
    }
    …
    if ((myScore - opponentsScore) > 10) {
        …
    }
    if ((100 - myScore) <= 20
            && Math.abs(myScore - opponentsScore) <= 10) {
        …
    }
    return true;

In fact, if you eliminate the Math.abs(), then you are redundantly restating the (100 - myScore) <= 20 && (myScore - opponentsScore) <= 10 case exactly.

Verbosity

Anytime you have if (condition) return true; else return false; you should just write return condition;.

I'm not a fan of needless arithmetic like if (100 - opponentsScore <= 20). It would be shorter to write if (opponentScore >= 80), and just as clear.

Interface

The function name could be simplified to shouldRoll.

Assuming a fair die, there is no logical strategy in which the turnNumber or the rollNumber could be a factor in the decision. It doesn't matter how you arrived at the current score and pool, because past performance is no indicator of future expectations. Anything else would be a gambler's fallacy. Therefore, I suggest eliminating turnNumber and rollNumber from the method signature.

I suggest renaming opponentsScore to theirScore, both to save a few characters and to mirror myScore.

Rewrite

There is too much code altogether! You are really just trying to express a simple idea: in the face of an imminent loss, go for a pool of 30 points; if you have a comfortable lead, aim for just 15; otherwise, try to gather 20.

@Override
public boolean shouldRoll(int poolSize, int myScore, int theirScore) {
    if (myScore + poolSize >= 100) {
        return false;
    }
    int myLead = myScore - theirScore;
    int wantPool = (myScore < 80 && theirScore >= 80) ? 30              // Imminent loss, be aggressive.
                 : (myScore >= 80 && (0 < myLead && myLead <= 10)) ? 15 // Comfortable lead
                 : 20;                                                  // General case
    return poolSize < wantPool;
}
\$\endgroup\$
  • \$\begingroup\$ Thank you!! This really helped understand the errors in my code AND the errors in my way of thinking of the problem. \$\endgroup\$ – Wiz Feb 14 '17 at 6:07

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.