0
public static double tripleBet(Dice dice, double betAmount) {
    double payout = 0.0;
    double three_rolled = 3;

    if (dice.getFirst() == dice.getSecond() && dice.getThird() == dice.getFirst()) {
        payout = betAmount * three_rolled;
    } else {
        payout = -betAmount;
    }
    return payout;

}

Here I am comparing die in a game called "Chuck-a-luck." I need to simply return a payout amount if the player has bet the dice will all be the same.

The expression in the conditional statement is my main concern. I'd like to know if it is valid or "good practice" to write like this.

Any other advice is welcomed as well.

  • 3
    Not an answer to your question but an important note: You are using floating point for currency values. This is _guaranteed_ to give you unexpected results at some point. [What Every Computer Scientist Should Know About Floating-Point Arithmetic](https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html) and [Is Floating Point Broken?](https://stackoverflow.com/q/588004/18157) – Jim Garrison Jun 20 '18 at 03:05
  • What's the type returned by `getFirst()` etc.? If anything other than a primitive, you should use `equals()` instead of `==`. – Robby Cornelissen Jun 20 '18 at 03:09
  • @RobbyCornelissen integers. – Evan Ross Davis Jun 20 '18 at 03:10
  • The if condition is fine. There is no really nice way to express it. – cpp beginner Jun 20 '18 at 03:10
  • You're fine then. Changing the order might make it a bit more readable (`1 == 2 && 2 == 3` instead of `1 == 2 && 3 == 1`), but that's about all you can do. – Robby Cornelissen Jun 20 '18 at 03:13
  • 1
    @JimGarrison non-base 10 numbering systems can cause some problems it looks like. – Evan Ross Davis Jun 20 '18 at 03:14
  • For money, yes, because money is accounted for in base 10. – user207421 Jun 20 '18 at 03:32

3 Answers3

2

Yes, it is valid. The == operator is transitive, meaning that A == B and B == C implies that A == C.

I would therefore probably write it as

if (dice.getFirst() == dice.getSecond() && dice.getSecond() == dice.getThird())
user207421
  • 305,947
  • 44
  • 307
  • 483
1

What you are doing is fine. It is also possible to write your own helper method for this.

@SafeVarargs
public static final boolean equals(Object... objs) {
    if (objs== null || objs.length < 2) return false; // You may return true or throw exception

    for (int i = 0; i < nums.length - 1; i++) {
        if (!Objects.equals(objs[i], objs[i + 1])) return false;
    }

    return true;
}

This will make it easier for you to read later on, provided you may have to a use-case where you compare even more values.

 if (Helper.equals(dice.getFirst(), dice.getSecond(), dice.getThird()) {}
Jai
  • 8,165
  • 2
  • 21
  • 52
  • 2
    The `if` condition in your loop should be negated. Also, for 0 and 1 arguments, it would be more typical to return `true`. See e.g. https://en.wikipedia.org/wiki/Vacuous_truth for formal reasons. – Radiodef Jun 20 '18 at 03:45
0

So far, I don't see any problems with the code you have given.

Here are some cosmetic updates you can make to your code. Will make it look much simpler and reduce the number of lines and save you some memory as well in the process.

public static double tripleBet(Dice dice, double betAmount) {

    double three_rolled = 3;

    // Using Ternary Operator we eliminated the need for a separate variable "payout" by simple returning the resultant values to the caller method.

    return (dice.getFirst() == dice.getSecond() && dice.getThird() == dice.getFirst()) ? betAmount * three_rolled : -betAmount;
}

P.S : If the value of the variable three_rolled will always stay as 3 then i think you can either assign it to a byte or a similar data type. Don't need a double for a value this small. Better memory management leads to a happy compiler and cleaner codes.

  • Why use a variable at all? Something wrong with `3`? – user207421 Jun 20 '18 at 08:17
  • Yeah 3 is fine, but using direct values is not recommended, Plus **just in case** `three_rolled` needs to be changed in the future then, it would be impossible to do so if we simply replace a variable with a fixed value. This is why i mentioned **IF** in my *PS* – 404 Brain Not Found Jun 20 '18 at 09:02
  • I’ll be testing everything today and have been thinking a lot about tips I’ve seen. – Evan Ross Davis Jun 20 '18 at 16:00
  • 'Using direct values is not recommended' *by whom*? Why? If you mean it should be a manifest constant, I would agree, but the code you posted doesn't exhibit that. It should be `public`, `static` and `final` to qualify as a manifest constant. Not a mutable local variable. – user207421 Jun 21 '18 at 10:06