4
\$\begingroup\$

I'm trying to make my code smaller and less verbose.

The pass in the if statement are there because I need to check specifics on the dice roll.

The code is running a game of craps where on the first roll if I get a 7 or 11 I win, or if I get a 2,3,12 I lose. Any other number I keep rolling till I either get what I rolled the first time or I roll a 7. A return value of 1 means I won and 0 means I lost.

def craps():
    from random import randint
    dice = 0
    loop = 0
    while loop < 1:
        d1 = randint(1,6)
        d2 = randint(1,6)
        roll = d1 + d2
        dice += 1
        print(d1, d2)
        if dice == 1:
            first=roll
            if roll in {2,3,12}:
                loop += 1
                return 0
            elif roll in {7,11}:
                loop += 1
                return 1
            else:
                pass
        elif dice != 1:
            if first==roll:
                loop += 1
                return 1
            elif roll==7:
                loop += 1
                return 0
            else:
                pass
        else:
            pass
\$\endgroup\$
5
\$\begingroup\$
  • loop is totally useless. The loop doesn't have a chance to test it: as soon as it becomes 1, the function returns.

  • Don't test for special cases inside the loop. A first roll is special, so roll it before the loop begins (this also eliminates a dice variable).

  • Factor dice rolling into a function.


That said,

    from random import randint

    def do_roll():
        return randint(1, 6) + randint(1, 6)

    def craps():
        first = do_roll()
        if first in {2,3,12}:
            return 0
        elif first in {7,11}:
            return 1

        while True:
            roll = do_roll()
            if roll == first:
                return 1
            elif roll == 7:
                return 0
\$\endgroup\$
  • \$\begingroup\$ Works good! only thing now is I need it to print the whatever the first and second roll was ( like the print statement in my code ). and ive tried modifying your code but no luck so far \$\endgroup\$ – George Jacob Flamburis Nov 1 '16 at 22:55
  • \$\begingroup\$ disregard my comment above, I've figured it out by adding the print(d1,d2) into the do_roll() and assigning the randints to a variable. \$\endgroup\$ – George Jacob Flamburis Nov 1 '16 at 23:07
  • \$\begingroup\$ You don't need if...elif...else pass, just if...elif \$\endgroup\$ – OldBunny2800 Nov 2 '16 at 1:02
3
\$\begingroup\$

On top vnp's answer, I would return True and False instead of 1 and 0 unless you have good reason to do otherwise. (Which you might, i.e. in the case you want to run a simulation. Otherwise, returning 1 and 0 instead of True and False is un-Pythonic.)


I would also add a docstring to each method (there are 2 in vnp's solution). So explain that:

  1. roll_dice returns the result of rolling two dice. Be sure to not say that it is the sum of two random numbers. Provide the context for doing that.

  2. Explain the rules of craps inside of the craps function (in the docstring).


(This section is less of a "review" and more of considerations for future development)

Not exactly required, but a lot of casinos have different variants on craps here are some possible abstractions you may want to consider:

  1. The two sets you use {2, 3, 12}, and {7, 11}. Maybe you should allow the user to supply such numbers?
  2. Allow one to change the sides of the dice.
\$\endgroup\$
  • \$\begingroup\$ i've used a 1 and 0 because in another function i'm calculating the odds of winning x number of games, and whatever number it returns i just add that onto an accumulator. i could probably use an if true...but that's for a later time to mess around with. but thank you for your help and ill look into your other suggestions, right now im just assuming who ever plays this game knows the rules of craps. \$\endgroup\$ – George Jacob Flamburis Nov 1 '16 at 23:02

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.