Razzle Dazzle simulatorAirport SimulatorDistributed system simulatorCircuit SimulatorPHP Lottery SimulatorMonty Hall simulator in Python 3.xSimple game outcome simulatorMonty Hall Simulator - PythonGreed Dice Scoring Game expanded - Python KoansC++ Beginner Bunny ExerciseLotto simulator in Python

Print "N NE E SE S SW W NW"

Converting from CMYK to RGB (to work with it), then back to CMYK

Does a (nice) centerless group always have a centerless profinite completion?

Was planting UN flag on Moon ever discussed?

What should I be wary of when insurer is taking a lot of time to decide whether car is repairable or a total loss?

Could a person damage a jet airliner - from the outside - with their bare hands?

Difference between prepositions in "...killed during/in the war"

How was the airlock installed on the Space Shuttle mid deck?

Why is Na5 not played in this line of the French Defense, Advance Variation?

If I had a daughter who (is/were/was) cute, I would be very happy

Confused with atmospheric pressure equals plastic balloon’s inner pressure

How to befriend someone who doesn't like to talk?

Who is "He that flies" in Lord of the Rings?

Command of files and size

Why are MBA programs closing in the United States?

Grep Match and extract

Trying to get (more) accurate readings from thermistor (electronics, math, and code inside)

What would be the way to say "just saying" in German? (Not the literal translation)

Remove border lines of SRTM tiles rendered as hillshade

Is it a acceptable way to write a loss function in this form?

Why is the length of the Kelvin unit of temperature equal to that of the Celsius unit?

Why isn't Bash trap working if output is redirected to stdout?

So a part of my house disappeared... But not because of a chunk resetting

How do you play "tenth" chords on the guitar?



Razzle Dazzle simulator


Airport SimulatorDistributed system simulatorCircuit SimulatorPHP Lottery SimulatorMonty Hall simulator in Python 3.xSimple game outcome simulatorMonty Hall Simulator - PythonGreed Dice Scoring Game expanded - Python KoansC++ Beginner Bunny ExerciseLotto simulator in Python






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








10












$begingroup$


Inspired by the video from Scam Nation and James Grime from Numberphile, I tried to make a Razzle Dazzle simulator.



Razzle Dazzle is a scam in the form of a game. Per turn, the player pays a fee and throws 8 marbles onto a board, so they land in holes in the board. Each hole has a score from 1 to 6. Throwing 8 dice instead can also be done. The scores are added to form a score from 8 to 48. This score is translated into points via table/chart. The points are accumulated across turns. When the player reaches 100 points, it wins a prize. Some scores increase the number of prizes when 100 points are reached. A score of 29 doubles the fee per turn, multiplicatively, so scoring 29 10 times increases the fee to 1024x the initial fee.



enter image description here



The trick is that the most common scores (22-34) do not give any points. This means that only 2.7% of the turns by fair dice rolls give out points, needing 369.5 turns to reach 100 points. For the board in the video, only 0.28% give points, resulting in 5000+ turns to get 100 points. The probability to score 29 is about 8%, this leads to massive fees when playing lots of turns.



import random, numpy
import matplotlib.pyplot as plt

# return one int with random value [1,6], with the probability density described in rawMassDist
# every 1000 turns, sample 1000 loaded die throws and put them in a list
randoms = []
idxRandom = 0
def throwLoadedDie():
global idxRandom
global randoms
rawMassDist = [11, 17, 39, 44, 21, 11]
#rawMassDist = [50, 5, 5, 5, 5, 50]
massDist = [float(i)/sum(rawMassDist) for i in rawMassDist]
if (idxRandom % 1000) == 0:
#randoms = numpy.random.choice(range(1, 7), size=1000, p=massDist)
randoms = random.choices(range(1,7), massDist, k=1000)
idxRandom = 0
idxRandom += 1
return randoms[idxRandom-1]

# throw 8 dice, fairDice indicates whether fair dice or loaded dice are used
# returns the sum of the dice values, which equals the score for this turn
def throwDice():
total = 0
for _ in range(0,8):
if fairDice:
total += random.randint(1,6);
else:
total += throwLoadedDie()
return total

# translates the score into points using dictionary toPoints
def getPoints(score):
toPoints = 8:100, 9:100, 10:50, 11:30, 12:50,
13:50, 14:20, 15:15, 16:10, 17:5,
39:5, 40:5, 41:15, 42:20, 43:50,
44:50, 45:50, 46:50, 47:50, 48:100
if score in toPoints:
return toPoints[score]
return 0

# returns if this score results in an extra price
def isExtraPrize(score):
if (18 <= score <= 21) or (score == 29) or (35 <= score <= 38):
return True
return False

# returns if this score doubles the fee for one turn
def needDoubleFee(score):
return score == 29

# simulate one turn, return the new number of points, prizes and fee for the next turn
def simulateTurn(points, prizes, fee):
score = throwDice()
if isExtraPrize(score):
prizes += 1
if needDoubleFee(score):
fee *= 2
points += getPoints(score)
return [points, prizes, fee, score]

# simulate single game, can result in win or loss in maxTurns turns
# can print result and histogram of scores
def playGame(printResult = True, maxTurns = 1000):
points = 0
prizes = 1
hist = list() # start with empty list, add score after every turn
hist2 = [0]*49 # entries 0-7 is always 0, other entries 8-48 represent the number of times a score has occurred
fee = 1
totalFee = 0
goal = 100
won = False
for turn in range(1, maxTurns+1):
#print('Turn 0, points: 1'.format(turn, points))
totalFee += fee
[points, prizes, fee, score] = simulateTurn(points, prizes, fee)
hist.append(score)
if points >= goal:
won = True
break

# finalize
[hist2, _] = numpy.histogram(hist, bins=49, range=[0,48])
if printResult:
if won:
print('You win 0 prizes in 1 turns, cost: 2'.format(prizes, turn, totalFee))
else:
print('You only got 0 points in 1 turns, cost: 2'.format(points, turn, totalFee))

print(hist2)
if not won:
prizes = 0
return [prizes, turn, totalFee, hist2]

# simulate multiple games, allow many turns per game to practically ensure win
# also disable result printing in each game
def playGames(numGames, plot=False):
hist = [0]*49
totalPrizes = 0
totalTurns = 0
totalFee = 0
withPoints = 0
gamesLost = 0
for i in range(0, numGames):
[prizes, turns, fee, hist2] = playGame(False, 100000)
if prizes == 0:
gamesLost += 1
hist = [x + y for x, y in zip(hist, hist2)]
totalPrizes += prizes
totalFee += fee
totalTurns += turns
for i in range(8, 18):
withPoints += hist[i]
for i in range(39, 49):
withPoints += hist[i]
print('0 games, lost 1'.format(numGames, gamesLost))
print('Avg prizes: '.format(totalPrizes/numGames))
print('Avg turns: '.format(totalTurns/numGames))
print('Avg fee: '.format(totalFee/numGames))
print(hist)
print('Percentage turns with points: :.2f'.format(100.0*withPoints/sum(hist)))

if plot:
# create list of colors to color each bar differently
colors = [item for sublist in [['red']*18, ['blue']*21, ['red']*10] for item in sublist]
plt.bar(range(0, 49), hist, color=colors)
plt.title('Score distribution across multiple games')
plt.xlabel('Score = sum of 8 dice')
plt.ylabel('Number of turns')
plt.text(40, 0.6*max(hist), 'Red barsngive points')
plt.show()

fairDice = False
#playGame()
playGames(100, plot=True)


Concrete questions:

1. Since calling random.choices() has some overhead, I generate 1000 loaded die rolls and put it in a global array. Is there a better of doing this without classes? In C I'd probably use static variables

2. To generate a histogram of all the scores during a game, I append to a list every turn, and then generate the histogram. Is this efficient performance-wise?

3. How are my names? Especially hist, hist2, isExtraPrize() and needDoubleFee()

4. My Ryzen 5 2400G with 3200 MHz RAM takes about 15s to simulate 100 loaded games, averaging
3550 turns per game. I somehow feel like this should be faster, any performance related suggestions are welcome

5. And of course, general code review answers are welcome










share|improve this question









$endgroup$


















    10












    $begingroup$


    Inspired by the video from Scam Nation and James Grime from Numberphile, I tried to make a Razzle Dazzle simulator.



    Razzle Dazzle is a scam in the form of a game. Per turn, the player pays a fee and throws 8 marbles onto a board, so they land in holes in the board. Each hole has a score from 1 to 6. Throwing 8 dice instead can also be done. The scores are added to form a score from 8 to 48. This score is translated into points via table/chart. The points are accumulated across turns. When the player reaches 100 points, it wins a prize. Some scores increase the number of prizes when 100 points are reached. A score of 29 doubles the fee per turn, multiplicatively, so scoring 29 10 times increases the fee to 1024x the initial fee.



    enter image description here



    The trick is that the most common scores (22-34) do not give any points. This means that only 2.7% of the turns by fair dice rolls give out points, needing 369.5 turns to reach 100 points. For the board in the video, only 0.28% give points, resulting in 5000+ turns to get 100 points. The probability to score 29 is about 8%, this leads to massive fees when playing lots of turns.



    import random, numpy
    import matplotlib.pyplot as plt

    # return one int with random value [1,6], with the probability density described in rawMassDist
    # every 1000 turns, sample 1000 loaded die throws and put them in a list
    randoms = []
    idxRandom = 0
    def throwLoadedDie():
    global idxRandom
    global randoms
    rawMassDist = [11, 17, 39, 44, 21, 11]
    #rawMassDist = [50, 5, 5, 5, 5, 50]
    massDist = [float(i)/sum(rawMassDist) for i in rawMassDist]
    if (idxRandom % 1000) == 0:
    #randoms = numpy.random.choice(range(1, 7), size=1000, p=massDist)
    randoms = random.choices(range(1,7), massDist, k=1000)
    idxRandom = 0
    idxRandom += 1
    return randoms[idxRandom-1]

    # throw 8 dice, fairDice indicates whether fair dice or loaded dice are used
    # returns the sum of the dice values, which equals the score for this turn
    def throwDice():
    total = 0
    for _ in range(0,8):
    if fairDice:
    total += random.randint(1,6);
    else:
    total += throwLoadedDie()
    return total

    # translates the score into points using dictionary toPoints
    def getPoints(score):
    toPoints = 8:100, 9:100, 10:50, 11:30, 12:50,
    13:50, 14:20, 15:15, 16:10, 17:5,
    39:5, 40:5, 41:15, 42:20, 43:50,
    44:50, 45:50, 46:50, 47:50, 48:100
    if score in toPoints:
    return toPoints[score]
    return 0

    # returns if this score results in an extra price
    def isExtraPrize(score):
    if (18 <= score <= 21) or (score == 29) or (35 <= score <= 38):
    return True
    return False

    # returns if this score doubles the fee for one turn
    def needDoubleFee(score):
    return score == 29

    # simulate one turn, return the new number of points, prizes and fee for the next turn
    def simulateTurn(points, prizes, fee):
    score = throwDice()
    if isExtraPrize(score):
    prizes += 1
    if needDoubleFee(score):
    fee *= 2
    points += getPoints(score)
    return [points, prizes, fee, score]

    # simulate single game, can result in win or loss in maxTurns turns
    # can print result and histogram of scores
    def playGame(printResult = True, maxTurns = 1000):
    points = 0
    prizes = 1
    hist = list() # start with empty list, add score after every turn
    hist2 = [0]*49 # entries 0-7 is always 0, other entries 8-48 represent the number of times a score has occurred
    fee = 1
    totalFee = 0
    goal = 100
    won = False
    for turn in range(1, maxTurns+1):
    #print('Turn 0, points: 1'.format(turn, points))
    totalFee += fee
    [points, prizes, fee, score] = simulateTurn(points, prizes, fee)
    hist.append(score)
    if points >= goal:
    won = True
    break

    # finalize
    [hist2, _] = numpy.histogram(hist, bins=49, range=[0,48])
    if printResult:
    if won:
    print('You win 0 prizes in 1 turns, cost: 2'.format(prizes, turn, totalFee))
    else:
    print('You only got 0 points in 1 turns, cost: 2'.format(points, turn, totalFee))

    print(hist2)
    if not won:
    prizes = 0
    return [prizes, turn, totalFee, hist2]

    # simulate multiple games, allow many turns per game to practically ensure win
    # also disable result printing in each game
    def playGames(numGames, plot=False):
    hist = [0]*49
    totalPrizes = 0
    totalTurns = 0
    totalFee = 0
    withPoints = 0
    gamesLost = 0
    for i in range(0, numGames):
    [prizes, turns, fee, hist2] = playGame(False, 100000)
    if prizes == 0:
    gamesLost += 1
    hist = [x + y for x, y in zip(hist, hist2)]
    totalPrizes += prizes
    totalFee += fee
    totalTurns += turns
    for i in range(8, 18):
    withPoints += hist[i]
    for i in range(39, 49):
    withPoints += hist[i]
    print('0 games, lost 1'.format(numGames, gamesLost))
    print('Avg prizes: '.format(totalPrizes/numGames))
    print('Avg turns: '.format(totalTurns/numGames))
    print('Avg fee: '.format(totalFee/numGames))
    print(hist)
    print('Percentage turns with points: :.2f'.format(100.0*withPoints/sum(hist)))

    if plot:
    # create list of colors to color each bar differently
    colors = [item for sublist in [['red']*18, ['blue']*21, ['red']*10] for item in sublist]
    plt.bar(range(0, 49), hist, color=colors)
    plt.title('Score distribution across multiple games')
    plt.xlabel('Score = sum of 8 dice')
    plt.ylabel('Number of turns')
    plt.text(40, 0.6*max(hist), 'Red barsngive points')
    plt.show()

    fairDice = False
    #playGame()
    playGames(100, plot=True)


    Concrete questions:

    1. Since calling random.choices() has some overhead, I generate 1000 loaded die rolls and put it in a global array. Is there a better of doing this without classes? In C I'd probably use static variables

    2. To generate a histogram of all the scores during a game, I append to a list every turn, and then generate the histogram. Is this efficient performance-wise?

    3. How are my names? Especially hist, hist2, isExtraPrize() and needDoubleFee()

    4. My Ryzen 5 2400G with 3200 MHz RAM takes about 15s to simulate 100 loaded games, averaging
    3550 turns per game. I somehow feel like this should be faster, any performance related suggestions are welcome

    5. And of course, general code review answers are welcome










    share|improve this question









    $endgroup$














      10












      10








      10


      2



      $begingroup$


      Inspired by the video from Scam Nation and James Grime from Numberphile, I tried to make a Razzle Dazzle simulator.



      Razzle Dazzle is a scam in the form of a game. Per turn, the player pays a fee and throws 8 marbles onto a board, so they land in holes in the board. Each hole has a score from 1 to 6. Throwing 8 dice instead can also be done. The scores are added to form a score from 8 to 48. This score is translated into points via table/chart. The points are accumulated across turns. When the player reaches 100 points, it wins a prize. Some scores increase the number of prizes when 100 points are reached. A score of 29 doubles the fee per turn, multiplicatively, so scoring 29 10 times increases the fee to 1024x the initial fee.



      enter image description here



      The trick is that the most common scores (22-34) do not give any points. This means that only 2.7% of the turns by fair dice rolls give out points, needing 369.5 turns to reach 100 points. For the board in the video, only 0.28% give points, resulting in 5000+ turns to get 100 points. The probability to score 29 is about 8%, this leads to massive fees when playing lots of turns.



      import random, numpy
      import matplotlib.pyplot as plt

      # return one int with random value [1,6], with the probability density described in rawMassDist
      # every 1000 turns, sample 1000 loaded die throws and put them in a list
      randoms = []
      idxRandom = 0
      def throwLoadedDie():
      global idxRandom
      global randoms
      rawMassDist = [11, 17, 39, 44, 21, 11]
      #rawMassDist = [50, 5, 5, 5, 5, 50]
      massDist = [float(i)/sum(rawMassDist) for i in rawMassDist]
      if (idxRandom % 1000) == 0:
      #randoms = numpy.random.choice(range(1, 7), size=1000, p=massDist)
      randoms = random.choices(range(1,7), massDist, k=1000)
      idxRandom = 0
      idxRandom += 1
      return randoms[idxRandom-1]

      # throw 8 dice, fairDice indicates whether fair dice or loaded dice are used
      # returns the sum of the dice values, which equals the score for this turn
      def throwDice():
      total = 0
      for _ in range(0,8):
      if fairDice:
      total += random.randint(1,6);
      else:
      total += throwLoadedDie()
      return total

      # translates the score into points using dictionary toPoints
      def getPoints(score):
      toPoints = 8:100, 9:100, 10:50, 11:30, 12:50,
      13:50, 14:20, 15:15, 16:10, 17:5,
      39:5, 40:5, 41:15, 42:20, 43:50,
      44:50, 45:50, 46:50, 47:50, 48:100
      if score in toPoints:
      return toPoints[score]
      return 0

      # returns if this score results in an extra price
      def isExtraPrize(score):
      if (18 <= score <= 21) or (score == 29) or (35 <= score <= 38):
      return True
      return False

      # returns if this score doubles the fee for one turn
      def needDoubleFee(score):
      return score == 29

      # simulate one turn, return the new number of points, prizes and fee for the next turn
      def simulateTurn(points, prizes, fee):
      score = throwDice()
      if isExtraPrize(score):
      prizes += 1
      if needDoubleFee(score):
      fee *= 2
      points += getPoints(score)
      return [points, prizes, fee, score]

      # simulate single game, can result in win or loss in maxTurns turns
      # can print result and histogram of scores
      def playGame(printResult = True, maxTurns = 1000):
      points = 0
      prizes = 1
      hist = list() # start with empty list, add score after every turn
      hist2 = [0]*49 # entries 0-7 is always 0, other entries 8-48 represent the number of times a score has occurred
      fee = 1
      totalFee = 0
      goal = 100
      won = False
      for turn in range(1, maxTurns+1):
      #print('Turn 0, points: 1'.format(turn, points))
      totalFee += fee
      [points, prizes, fee, score] = simulateTurn(points, prizes, fee)
      hist.append(score)
      if points >= goal:
      won = True
      break

      # finalize
      [hist2, _] = numpy.histogram(hist, bins=49, range=[0,48])
      if printResult:
      if won:
      print('You win 0 prizes in 1 turns, cost: 2'.format(prizes, turn, totalFee))
      else:
      print('You only got 0 points in 1 turns, cost: 2'.format(points, turn, totalFee))

      print(hist2)
      if not won:
      prizes = 0
      return [prizes, turn, totalFee, hist2]

      # simulate multiple games, allow many turns per game to practically ensure win
      # also disable result printing in each game
      def playGames(numGames, plot=False):
      hist = [0]*49
      totalPrizes = 0
      totalTurns = 0
      totalFee = 0
      withPoints = 0
      gamesLost = 0
      for i in range(0, numGames):
      [prizes, turns, fee, hist2] = playGame(False, 100000)
      if prizes == 0:
      gamesLost += 1
      hist = [x + y for x, y in zip(hist, hist2)]
      totalPrizes += prizes
      totalFee += fee
      totalTurns += turns
      for i in range(8, 18):
      withPoints += hist[i]
      for i in range(39, 49):
      withPoints += hist[i]
      print('0 games, lost 1'.format(numGames, gamesLost))
      print('Avg prizes: '.format(totalPrizes/numGames))
      print('Avg turns: '.format(totalTurns/numGames))
      print('Avg fee: '.format(totalFee/numGames))
      print(hist)
      print('Percentage turns with points: :.2f'.format(100.0*withPoints/sum(hist)))

      if plot:
      # create list of colors to color each bar differently
      colors = [item for sublist in [['red']*18, ['blue']*21, ['red']*10] for item in sublist]
      plt.bar(range(0, 49), hist, color=colors)
      plt.title('Score distribution across multiple games')
      plt.xlabel('Score = sum of 8 dice')
      plt.ylabel('Number of turns')
      plt.text(40, 0.6*max(hist), 'Red barsngive points')
      plt.show()

      fairDice = False
      #playGame()
      playGames(100, plot=True)


      Concrete questions:

      1. Since calling random.choices() has some overhead, I generate 1000 loaded die rolls and put it in a global array. Is there a better of doing this without classes? In C I'd probably use static variables

      2. To generate a histogram of all the scores during a game, I append to a list every turn, and then generate the histogram. Is this efficient performance-wise?

      3. How are my names? Especially hist, hist2, isExtraPrize() and needDoubleFee()

      4. My Ryzen 5 2400G with 3200 MHz RAM takes about 15s to simulate 100 loaded games, averaging
      3550 turns per game. I somehow feel like this should be faster, any performance related suggestions are welcome

      5. And of course, general code review answers are welcome










      share|improve this question









      $endgroup$




      Inspired by the video from Scam Nation and James Grime from Numberphile, I tried to make a Razzle Dazzle simulator.



      Razzle Dazzle is a scam in the form of a game. Per turn, the player pays a fee and throws 8 marbles onto a board, so they land in holes in the board. Each hole has a score from 1 to 6. Throwing 8 dice instead can also be done. The scores are added to form a score from 8 to 48. This score is translated into points via table/chart. The points are accumulated across turns. When the player reaches 100 points, it wins a prize. Some scores increase the number of prizes when 100 points are reached. A score of 29 doubles the fee per turn, multiplicatively, so scoring 29 10 times increases the fee to 1024x the initial fee.



      enter image description here



      The trick is that the most common scores (22-34) do not give any points. This means that only 2.7% of the turns by fair dice rolls give out points, needing 369.5 turns to reach 100 points. For the board in the video, only 0.28% give points, resulting in 5000+ turns to get 100 points. The probability to score 29 is about 8%, this leads to massive fees when playing lots of turns.



      import random, numpy
      import matplotlib.pyplot as plt

      # return one int with random value [1,6], with the probability density described in rawMassDist
      # every 1000 turns, sample 1000 loaded die throws and put them in a list
      randoms = []
      idxRandom = 0
      def throwLoadedDie():
      global idxRandom
      global randoms
      rawMassDist = [11, 17, 39, 44, 21, 11]
      #rawMassDist = [50, 5, 5, 5, 5, 50]
      massDist = [float(i)/sum(rawMassDist) for i in rawMassDist]
      if (idxRandom % 1000) == 0:
      #randoms = numpy.random.choice(range(1, 7), size=1000, p=massDist)
      randoms = random.choices(range(1,7), massDist, k=1000)
      idxRandom = 0
      idxRandom += 1
      return randoms[idxRandom-1]

      # throw 8 dice, fairDice indicates whether fair dice or loaded dice are used
      # returns the sum of the dice values, which equals the score for this turn
      def throwDice():
      total = 0
      for _ in range(0,8):
      if fairDice:
      total += random.randint(1,6);
      else:
      total += throwLoadedDie()
      return total

      # translates the score into points using dictionary toPoints
      def getPoints(score):
      toPoints = 8:100, 9:100, 10:50, 11:30, 12:50,
      13:50, 14:20, 15:15, 16:10, 17:5,
      39:5, 40:5, 41:15, 42:20, 43:50,
      44:50, 45:50, 46:50, 47:50, 48:100
      if score in toPoints:
      return toPoints[score]
      return 0

      # returns if this score results in an extra price
      def isExtraPrize(score):
      if (18 <= score <= 21) or (score == 29) or (35 <= score <= 38):
      return True
      return False

      # returns if this score doubles the fee for one turn
      def needDoubleFee(score):
      return score == 29

      # simulate one turn, return the new number of points, prizes and fee for the next turn
      def simulateTurn(points, prizes, fee):
      score = throwDice()
      if isExtraPrize(score):
      prizes += 1
      if needDoubleFee(score):
      fee *= 2
      points += getPoints(score)
      return [points, prizes, fee, score]

      # simulate single game, can result in win or loss in maxTurns turns
      # can print result and histogram of scores
      def playGame(printResult = True, maxTurns = 1000):
      points = 0
      prizes = 1
      hist = list() # start with empty list, add score after every turn
      hist2 = [0]*49 # entries 0-7 is always 0, other entries 8-48 represent the number of times a score has occurred
      fee = 1
      totalFee = 0
      goal = 100
      won = False
      for turn in range(1, maxTurns+1):
      #print('Turn 0, points: 1'.format(turn, points))
      totalFee += fee
      [points, prizes, fee, score] = simulateTurn(points, prizes, fee)
      hist.append(score)
      if points >= goal:
      won = True
      break

      # finalize
      [hist2, _] = numpy.histogram(hist, bins=49, range=[0,48])
      if printResult:
      if won:
      print('You win 0 prizes in 1 turns, cost: 2'.format(prizes, turn, totalFee))
      else:
      print('You only got 0 points in 1 turns, cost: 2'.format(points, turn, totalFee))

      print(hist2)
      if not won:
      prizes = 0
      return [prizes, turn, totalFee, hist2]

      # simulate multiple games, allow many turns per game to practically ensure win
      # also disable result printing in each game
      def playGames(numGames, plot=False):
      hist = [0]*49
      totalPrizes = 0
      totalTurns = 0
      totalFee = 0
      withPoints = 0
      gamesLost = 0
      for i in range(0, numGames):
      [prizes, turns, fee, hist2] = playGame(False, 100000)
      if prizes == 0:
      gamesLost += 1
      hist = [x + y for x, y in zip(hist, hist2)]
      totalPrizes += prizes
      totalFee += fee
      totalTurns += turns
      for i in range(8, 18):
      withPoints += hist[i]
      for i in range(39, 49):
      withPoints += hist[i]
      print('0 games, lost 1'.format(numGames, gamesLost))
      print('Avg prizes: '.format(totalPrizes/numGames))
      print('Avg turns: '.format(totalTurns/numGames))
      print('Avg fee: '.format(totalFee/numGames))
      print(hist)
      print('Percentage turns with points: :.2f'.format(100.0*withPoints/sum(hist)))

      if plot:
      # create list of colors to color each bar differently
      colors = [item for sublist in [['red']*18, ['blue']*21, ['red']*10] for item in sublist]
      plt.bar(range(0, 49), hist, color=colors)
      plt.title('Score distribution across multiple games')
      plt.xlabel('Score = sum of 8 dice')
      plt.ylabel('Number of turns')
      plt.text(40, 0.6*max(hist), 'Red barsngive points')
      plt.show()

      fairDice = False
      #playGame()
      playGames(100, plot=True)


      Concrete questions:

      1. Since calling random.choices() has some overhead, I generate 1000 loaded die rolls and put it in a global array. Is there a better of doing this without classes? In C I'd probably use static variables

      2. To generate a histogram of all the scores during a game, I append to a list every turn, and then generate the histogram. Is this efficient performance-wise?

      3. How are my names? Especially hist, hist2, isExtraPrize() and needDoubleFee()

      4. My Ryzen 5 2400G with 3200 MHz RAM takes about 15s to simulate 100 loaded games, averaging
      3550 turns per game. I somehow feel like this should be faster, any performance related suggestions are welcome

      5. And of course, general code review answers are welcome







      python performance simulation






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked 8 hours ago









      user2966394user2966394

      895




      895




















          1 Answer
          1






          active

          oldest

          votes


















          1












          $begingroup$

          First, your use of camelCase isn't ideal in Python. For variable and function names, snake_case is preferred. I'll be using that with any re-written code that I show.




          I think throw_dice can be improved a bit. You're checking for the value of fair_dice once per iteration in the function instead of once at the beginning. This will be negligible performance-wise, but it's unnecessary and checking once per loop suggests that it's a value that can change in the loop, which isn't the case here.



          There's different ways of approaching this depending on how close to PEP you want to adhere to; but both ways I'll show depend on dispatching to a function using a conditional expression. Following PEP, you could do something like:



          def throw_loaded_die():
          return 1 # For brevity

          # Break this off into its own function
          def throw_fair_die():
          return random.randint(1, 6)

          def throw_dice():
          # Figure out what we need first
          roll_f = throw_fair_die if fair_dice else throw_loaded_die

          total = 0
          for _ in range(8):
          total += roll_f() # Then use it here

          return total


          That cuts down on duplication which is nice. I also got rid of the 0 argument in the call to range as that's implicit if it isn't specified.



          I think the separate def throw_fair_die is unfortunate though. For such a simple function, I find it to be noisy, and looking around, I'm not the only one to feel this way. Personally, I'd prefer to just write:



          def throw_dice():
          # Notice the lambda
          roll_f = (lambda: random.randint(1, 6)) if fair_dice else throwLoadedDie

          total = 0
          for _ in range(8): # Specifying the start is unnecessary when it's 0
          total += roll_f()

          return total


          This is arguably a "named lambda" though, which is in violation of the recommendations of PEP:




          Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.




          ¯_(ツ)_/¯



          I still think it can be improved though. Look carefully at the loop. It's just a summing loop! Python has a built-in for that that can be used cleanly with a generator expression:



          def throw_dice():
          roll_f = throw_fair_die if fair_dice else throw_loaded_die

          return sum(roll_f() for _ in range(8))



          is_extra_prize has a redundant return. It can be simplified to:



          def is_extra_prize(score):
          return (18 <= score <= 21) or (score == 29) or (35 <= score <= 38)


          I'll point out though that right below it you have need_double_fee. Either it's justified to have score == 29 broken off into its own function (in which case it should be used in the appropriate cases), or it's not. If you feel the need to have it as a separate function, I'd use it:



          def need_double_fee(score):
          return score == 29

          def is_extra_prize(score):
          return (18 <= score <= 21) or need_double_fee(score) or (35 <= score <= 38)


          Although it could be argued that the other two parts of the condition in is_extra_prize are more complicated than score == 29, and may benefit from having a name attached to them as well. There's also the alternative of naming the 29 magic number directly, which I feel would probably be an even better option:



          EXTRA_PRIZE_SCORE = 29

          def is_extra_prize(score):
          return (18 <= score <= 21) or score == EXTRA_PRIZE_SCORE or (35 <= score <= 38)


          You may find naming 18, 21, 35 and 38 are beneficial as well; although that will certainly make that function more verbose.




          I think get_points can be improved as well. The score dictionary seems like it's a "member of the entire program", not something that should be local to the function. You can also use get on the dictionary to avoid the explicit membership lookup:



          SCORE_TO_POINTS = 8:100, 9:100, 10:50, 11:30, 12:50,
          13:50, 14:20, 15:15, 16:10, 17:5,
          39:5, 40:5, 41:15, 42:20, 43:50,
          44:50, 45:50, 46:50, 47:50, 48:100

          def get_points(score):
          # 0 is the default if the key doesn't exist
          return SCORE_TO_POINTS.get(score, 0)



          simulate_turn returns a tuple (actually a list, although it probably should be a tuple) representing the new state of the game. This is fine for simple states, but your current state has four pieces, and accessing them requires memorizing what order they're in, and allows mistakes to be made if data is placed incorrectly. You may want to look into using a class here for organization and clarity, or even a named tuple as a shortcut.



          In that same function, I'd also add some lines to space things out a bit:



          def simulate_turn(points, prizes, fee):
          score = throwDice()

          if isExtraPrize(score):
          prizes += 1

          if needDoubleFee(score):
          fee *= 2

          points += getPoints(score)

          return (points, prizes, fee, score)


          Personal style, but I like open space in code.



          You could also do away with the mutation of the parameters:



          def simulate_turn(points, prizes, fee):
          score = throwDice()

          return (points + getPoints(score),
          prizes + 1 if isExtraPrize(score) else prizes,
          fee * 2 if needDoubleFee(score) else fee,
          score)


          Although now that it's written out, I'm not sure how I feel about it.





          I really only dealt with 5. here. Hopefully someone else can touch on the first four points.






          share|improve this answer









          $endgroup$













            Your Answer






            StackExchange.ifUsing("editor", function ()
            StackExchange.using("externalEditor", function ()
            StackExchange.using("snippets", function ()
            StackExchange.snippets.init();
            );
            );
            , "code-snippets");

            StackExchange.ready(function()
            var channelOptions =
            tags: "".split(" "),
            id: "196"
            ;
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function()
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled)
            StackExchange.using("snippets", function()
            createEditor();
            );

            else
            createEditor();

            );

            function createEditor()
            StackExchange.prepareEditor(
            heartbeatType: 'answer',
            autoActivateHeartbeat: false,
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader:
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            ,
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            );



            );













            draft saved

            draft discarded


















            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221972%2frazzle-dazzle-simulator%23new-answer', 'question_page');

            );

            Post as a guest















            Required, but never shown

























            1 Answer
            1






            active

            oldest

            votes








            1 Answer
            1






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes









            1












            $begingroup$

            First, your use of camelCase isn't ideal in Python. For variable and function names, snake_case is preferred. I'll be using that with any re-written code that I show.




            I think throw_dice can be improved a bit. You're checking for the value of fair_dice once per iteration in the function instead of once at the beginning. This will be negligible performance-wise, but it's unnecessary and checking once per loop suggests that it's a value that can change in the loop, which isn't the case here.



            There's different ways of approaching this depending on how close to PEP you want to adhere to; but both ways I'll show depend on dispatching to a function using a conditional expression. Following PEP, you could do something like:



            def throw_loaded_die():
            return 1 # For brevity

            # Break this off into its own function
            def throw_fair_die():
            return random.randint(1, 6)

            def throw_dice():
            # Figure out what we need first
            roll_f = throw_fair_die if fair_dice else throw_loaded_die

            total = 0
            for _ in range(8):
            total += roll_f() # Then use it here

            return total


            That cuts down on duplication which is nice. I also got rid of the 0 argument in the call to range as that's implicit if it isn't specified.



            I think the separate def throw_fair_die is unfortunate though. For such a simple function, I find it to be noisy, and looking around, I'm not the only one to feel this way. Personally, I'd prefer to just write:



            def throw_dice():
            # Notice the lambda
            roll_f = (lambda: random.randint(1, 6)) if fair_dice else throwLoadedDie

            total = 0
            for _ in range(8): # Specifying the start is unnecessary when it's 0
            total += roll_f()

            return total


            This is arguably a "named lambda" though, which is in violation of the recommendations of PEP:




            Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.




            ¯_(ツ)_/¯



            I still think it can be improved though. Look carefully at the loop. It's just a summing loop! Python has a built-in for that that can be used cleanly with a generator expression:



            def throw_dice():
            roll_f = throw_fair_die if fair_dice else throw_loaded_die

            return sum(roll_f() for _ in range(8))



            is_extra_prize has a redundant return. It can be simplified to:



            def is_extra_prize(score):
            return (18 <= score <= 21) or (score == 29) or (35 <= score <= 38)


            I'll point out though that right below it you have need_double_fee. Either it's justified to have score == 29 broken off into its own function (in which case it should be used in the appropriate cases), or it's not. If you feel the need to have it as a separate function, I'd use it:



            def need_double_fee(score):
            return score == 29

            def is_extra_prize(score):
            return (18 <= score <= 21) or need_double_fee(score) or (35 <= score <= 38)


            Although it could be argued that the other two parts of the condition in is_extra_prize are more complicated than score == 29, and may benefit from having a name attached to them as well. There's also the alternative of naming the 29 magic number directly, which I feel would probably be an even better option:



            EXTRA_PRIZE_SCORE = 29

            def is_extra_prize(score):
            return (18 <= score <= 21) or score == EXTRA_PRIZE_SCORE or (35 <= score <= 38)


            You may find naming 18, 21, 35 and 38 are beneficial as well; although that will certainly make that function more verbose.




            I think get_points can be improved as well. The score dictionary seems like it's a "member of the entire program", not something that should be local to the function. You can also use get on the dictionary to avoid the explicit membership lookup:



            SCORE_TO_POINTS = 8:100, 9:100, 10:50, 11:30, 12:50,
            13:50, 14:20, 15:15, 16:10, 17:5,
            39:5, 40:5, 41:15, 42:20, 43:50,
            44:50, 45:50, 46:50, 47:50, 48:100

            def get_points(score):
            # 0 is the default if the key doesn't exist
            return SCORE_TO_POINTS.get(score, 0)



            simulate_turn returns a tuple (actually a list, although it probably should be a tuple) representing the new state of the game. This is fine for simple states, but your current state has four pieces, and accessing them requires memorizing what order they're in, and allows mistakes to be made if data is placed incorrectly. You may want to look into using a class here for organization and clarity, or even a named tuple as a shortcut.



            In that same function, I'd also add some lines to space things out a bit:



            def simulate_turn(points, prizes, fee):
            score = throwDice()

            if isExtraPrize(score):
            prizes += 1

            if needDoubleFee(score):
            fee *= 2

            points += getPoints(score)

            return (points, prizes, fee, score)


            Personal style, but I like open space in code.



            You could also do away with the mutation of the parameters:



            def simulate_turn(points, prizes, fee):
            score = throwDice()

            return (points + getPoints(score),
            prizes + 1 if isExtraPrize(score) else prizes,
            fee * 2 if needDoubleFee(score) else fee,
            score)


            Although now that it's written out, I'm not sure how I feel about it.





            I really only dealt with 5. here. Hopefully someone else can touch on the first four points.






            share|improve this answer









            $endgroup$

















              1












              $begingroup$

              First, your use of camelCase isn't ideal in Python. For variable and function names, snake_case is preferred. I'll be using that with any re-written code that I show.




              I think throw_dice can be improved a bit. You're checking for the value of fair_dice once per iteration in the function instead of once at the beginning. This will be negligible performance-wise, but it's unnecessary and checking once per loop suggests that it's a value that can change in the loop, which isn't the case here.



              There's different ways of approaching this depending on how close to PEP you want to adhere to; but both ways I'll show depend on dispatching to a function using a conditional expression. Following PEP, you could do something like:



              def throw_loaded_die():
              return 1 # For brevity

              # Break this off into its own function
              def throw_fair_die():
              return random.randint(1, 6)

              def throw_dice():
              # Figure out what we need first
              roll_f = throw_fair_die if fair_dice else throw_loaded_die

              total = 0
              for _ in range(8):
              total += roll_f() # Then use it here

              return total


              That cuts down on duplication which is nice. I also got rid of the 0 argument in the call to range as that's implicit if it isn't specified.



              I think the separate def throw_fair_die is unfortunate though. For such a simple function, I find it to be noisy, and looking around, I'm not the only one to feel this way. Personally, I'd prefer to just write:



              def throw_dice():
              # Notice the lambda
              roll_f = (lambda: random.randint(1, 6)) if fair_dice else throwLoadedDie

              total = 0
              for _ in range(8): # Specifying the start is unnecessary when it's 0
              total += roll_f()

              return total


              This is arguably a "named lambda" though, which is in violation of the recommendations of PEP:




              Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.




              ¯_(ツ)_/¯



              I still think it can be improved though. Look carefully at the loop. It's just a summing loop! Python has a built-in for that that can be used cleanly with a generator expression:



              def throw_dice():
              roll_f = throw_fair_die if fair_dice else throw_loaded_die

              return sum(roll_f() for _ in range(8))



              is_extra_prize has a redundant return. It can be simplified to:



              def is_extra_prize(score):
              return (18 <= score <= 21) or (score == 29) or (35 <= score <= 38)


              I'll point out though that right below it you have need_double_fee. Either it's justified to have score == 29 broken off into its own function (in which case it should be used in the appropriate cases), or it's not. If you feel the need to have it as a separate function, I'd use it:



              def need_double_fee(score):
              return score == 29

              def is_extra_prize(score):
              return (18 <= score <= 21) or need_double_fee(score) or (35 <= score <= 38)


              Although it could be argued that the other two parts of the condition in is_extra_prize are more complicated than score == 29, and may benefit from having a name attached to them as well. There's also the alternative of naming the 29 magic number directly, which I feel would probably be an even better option:



              EXTRA_PRIZE_SCORE = 29

              def is_extra_prize(score):
              return (18 <= score <= 21) or score == EXTRA_PRIZE_SCORE or (35 <= score <= 38)


              You may find naming 18, 21, 35 and 38 are beneficial as well; although that will certainly make that function more verbose.




              I think get_points can be improved as well. The score dictionary seems like it's a "member of the entire program", not something that should be local to the function. You can also use get on the dictionary to avoid the explicit membership lookup:



              SCORE_TO_POINTS = 8:100, 9:100, 10:50, 11:30, 12:50,
              13:50, 14:20, 15:15, 16:10, 17:5,
              39:5, 40:5, 41:15, 42:20, 43:50,
              44:50, 45:50, 46:50, 47:50, 48:100

              def get_points(score):
              # 0 is the default if the key doesn't exist
              return SCORE_TO_POINTS.get(score, 0)



              simulate_turn returns a tuple (actually a list, although it probably should be a tuple) representing the new state of the game. This is fine for simple states, but your current state has four pieces, and accessing them requires memorizing what order they're in, and allows mistakes to be made if data is placed incorrectly. You may want to look into using a class here for organization and clarity, or even a named tuple as a shortcut.



              In that same function, I'd also add some lines to space things out a bit:



              def simulate_turn(points, prizes, fee):
              score = throwDice()

              if isExtraPrize(score):
              prizes += 1

              if needDoubleFee(score):
              fee *= 2

              points += getPoints(score)

              return (points, prizes, fee, score)


              Personal style, but I like open space in code.



              You could also do away with the mutation of the parameters:



              def simulate_turn(points, prizes, fee):
              score = throwDice()

              return (points + getPoints(score),
              prizes + 1 if isExtraPrize(score) else prizes,
              fee * 2 if needDoubleFee(score) else fee,
              score)


              Although now that it's written out, I'm not sure how I feel about it.





              I really only dealt with 5. here. Hopefully someone else can touch on the first four points.






              share|improve this answer









              $endgroup$















                1












                1








                1





                $begingroup$

                First, your use of camelCase isn't ideal in Python. For variable and function names, snake_case is preferred. I'll be using that with any re-written code that I show.




                I think throw_dice can be improved a bit. You're checking for the value of fair_dice once per iteration in the function instead of once at the beginning. This will be negligible performance-wise, but it's unnecessary and checking once per loop suggests that it's a value that can change in the loop, which isn't the case here.



                There's different ways of approaching this depending on how close to PEP you want to adhere to; but both ways I'll show depend on dispatching to a function using a conditional expression. Following PEP, you could do something like:



                def throw_loaded_die():
                return 1 # For brevity

                # Break this off into its own function
                def throw_fair_die():
                return random.randint(1, 6)

                def throw_dice():
                # Figure out what we need first
                roll_f = throw_fair_die if fair_dice else throw_loaded_die

                total = 0
                for _ in range(8):
                total += roll_f() # Then use it here

                return total


                That cuts down on duplication which is nice. I also got rid of the 0 argument in the call to range as that's implicit if it isn't specified.



                I think the separate def throw_fair_die is unfortunate though. For such a simple function, I find it to be noisy, and looking around, I'm not the only one to feel this way. Personally, I'd prefer to just write:



                def throw_dice():
                # Notice the lambda
                roll_f = (lambda: random.randint(1, 6)) if fair_dice else throwLoadedDie

                total = 0
                for _ in range(8): # Specifying the start is unnecessary when it's 0
                total += roll_f()

                return total


                This is arguably a "named lambda" though, which is in violation of the recommendations of PEP:




                Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.




                ¯_(ツ)_/¯



                I still think it can be improved though. Look carefully at the loop. It's just a summing loop! Python has a built-in for that that can be used cleanly with a generator expression:



                def throw_dice():
                roll_f = throw_fair_die if fair_dice else throw_loaded_die

                return sum(roll_f() for _ in range(8))



                is_extra_prize has a redundant return. It can be simplified to:



                def is_extra_prize(score):
                return (18 <= score <= 21) or (score == 29) or (35 <= score <= 38)


                I'll point out though that right below it you have need_double_fee. Either it's justified to have score == 29 broken off into its own function (in which case it should be used in the appropriate cases), or it's not. If you feel the need to have it as a separate function, I'd use it:



                def need_double_fee(score):
                return score == 29

                def is_extra_prize(score):
                return (18 <= score <= 21) or need_double_fee(score) or (35 <= score <= 38)


                Although it could be argued that the other two parts of the condition in is_extra_prize are more complicated than score == 29, and may benefit from having a name attached to them as well. There's also the alternative of naming the 29 magic number directly, which I feel would probably be an even better option:



                EXTRA_PRIZE_SCORE = 29

                def is_extra_prize(score):
                return (18 <= score <= 21) or score == EXTRA_PRIZE_SCORE or (35 <= score <= 38)


                You may find naming 18, 21, 35 and 38 are beneficial as well; although that will certainly make that function more verbose.




                I think get_points can be improved as well. The score dictionary seems like it's a "member of the entire program", not something that should be local to the function. You can also use get on the dictionary to avoid the explicit membership lookup:



                SCORE_TO_POINTS = 8:100, 9:100, 10:50, 11:30, 12:50,
                13:50, 14:20, 15:15, 16:10, 17:5,
                39:5, 40:5, 41:15, 42:20, 43:50,
                44:50, 45:50, 46:50, 47:50, 48:100

                def get_points(score):
                # 0 is the default if the key doesn't exist
                return SCORE_TO_POINTS.get(score, 0)



                simulate_turn returns a tuple (actually a list, although it probably should be a tuple) representing the new state of the game. This is fine for simple states, but your current state has four pieces, and accessing them requires memorizing what order they're in, and allows mistakes to be made if data is placed incorrectly. You may want to look into using a class here for organization and clarity, or even a named tuple as a shortcut.



                In that same function, I'd also add some lines to space things out a bit:



                def simulate_turn(points, prizes, fee):
                score = throwDice()

                if isExtraPrize(score):
                prizes += 1

                if needDoubleFee(score):
                fee *= 2

                points += getPoints(score)

                return (points, prizes, fee, score)


                Personal style, but I like open space in code.



                You could also do away with the mutation of the parameters:



                def simulate_turn(points, prizes, fee):
                score = throwDice()

                return (points + getPoints(score),
                prizes + 1 if isExtraPrize(score) else prizes,
                fee * 2 if needDoubleFee(score) else fee,
                score)


                Although now that it's written out, I'm not sure how I feel about it.





                I really only dealt with 5. here. Hopefully someone else can touch on the first four points.






                share|improve this answer









                $endgroup$



                First, your use of camelCase isn't ideal in Python. For variable and function names, snake_case is preferred. I'll be using that with any re-written code that I show.




                I think throw_dice can be improved a bit. You're checking for the value of fair_dice once per iteration in the function instead of once at the beginning. This will be negligible performance-wise, but it's unnecessary and checking once per loop suggests that it's a value that can change in the loop, which isn't the case here.



                There's different ways of approaching this depending on how close to PEP you want to adhere to; but both ways I'll show depend on dispatching to a function using a conditional expression. Following PEP, you could do something like:



                def throw_loaded_die():
                return 1 # For brevity

                # Break this off into its own function
                def throw_fair_die():
                return random.randint(1, 6)

                def throw_dice():
                # Figure out what we need first
                roll_f = throw_fair_die if fair_dice else throw_loaded_die

                total = 0
                for _ in range(8):
                total += roll_f() # Then use it here

                return total


                That cuts down on duplication which is nice. I also got rid of the 0 argument in the call to range as that's implicit if it isn't specified.



                I think the separate def throw_fair_die is unfortunate though. For such a simple function, I find it to be noisy, and looking around, I'm not the only one to feel this way. Personally, I'd prefer to just write:



                def throw_dice():
                # Notice the lambda
                roll_f = (lambda: random.randint(1, 6)) if fair_dice else throwLoadedDie

                total = 0
                for _ in range(8): # Specifying the start is unnecessary when it's 0
                total += roll_f()

                return total


                This is arguably a "named lambda" though, which is in violation of the recommendations of PEP:




                Always use a def statement instead of an assignment statement that binds a lambda expression directly to an identifier.




                ¯_(ツ)_/¯



                I still think it can be improved though. Look carefully at the loop. It's just a summing loop! Python has a built-in for that that can be used cleanly with a generator expression:



                def throw_dice():
                roll_f = throw_fair_die if fair_dice else throw_loaded_die

                return sum(roll_f() for _ in range(8))



                is_extra_prize has a redundant return. It can be simplified to:



                def is_extra_prize(score):
                return (18 <= score <= 21) or (score == 29) or (35 <= score <= 38)


                I'll point out though that right below it you have need_double_fee. Either it's justified to have score == 29 broken off into its own function (in which case it should be used in the appropriate cases), or it's not. If you feel the need to have it as a separate function, I'd use it:



                def need_double_fee(score):
                return score == 29

                def is_extra_prize(score):
                return (18 <= score <= 21) or need_double_fee(score) or (35 <= score <= 38)


                Although it could be argued that the other two parts of the condition in is_extra_prize are more complicated than score == 29, and may benefit from having a name attached to them as well. There's also the alternative of naming the 29 magic number directly, which I feel would probably be an even better option:



                EXTRA_PRIZE_SCORE = 29

                def is_extra_prize(score):
                return (18 <= score <= 21) or score == EXTRA_PRIZE_SCORE or (35 <= score <= 38)


                You may find naming 18, 21, 35 and 38 are beneficial as well; although that will certainly make that function more verbose.




                I think get_points can be improved as well. The score dictionary seems like it's a "member of the entire program", not something that should be local to the function. You can also use get on the dictionary to avoid the explicit membership lookup:



                SCORE_TO_POINTS = 8:100, 9:100, 10:50, 11:30, 12:50,
                13:50, 14:20, 15:15, 16:10, 17:5,
                39:5, 40:5, 41:15, 42:20, 43:50,
                44:50, 45:50, 46:50, 47:50, 48:100

                def get_points(score):
                # 0 is the default if the key doesn't exist
                return SCORE_TO_POINTS.get(score, 0)



                simulate_turn returns a tuple (actually a list, although it probably should be a tuple) representing the new state of the game. This is fine for simple states, but your current state has four pieces, and accessing them requires memorizing what order they're in, and allows mistakes to be made if data is placed incorrectly. You may want to look into using a class here for organization and clarity, or even a named tuple as a shortcut.



                In that same function, I'd also add some lines to space things out a bit:



                def simulate_turn(points, prizes, fee):
                score = throwDice()

                if isExtraPrize(score):
                prizes += 1

                if needDoubleFee(score):
                fee *= 2

                points += getPoints(score)

                return (points, prizes, fee, score)


                Personal style, but I like open space in code.



                You could also do away with the mutation of the parameters:



                def simulate_turn(points, prizes, fee):
                score = throwDice()

                return (points + getPoints(score),
                prizes + 1 if isExtraPrize(score) else prizes,
                fee * 2 if needDoubleFee(score) else fee,
                score)


                Although now that it's written out, I'm not sure how I feel about it.





                I really only dealt with 5. here. Hopefully someone else can touch on the first four points.







                share|improve this answer












                share|improve this answer



                share|improve this answer










                answered 2 hours ago









                CarcigenicateCarcigenicate

                4,91111737




                4,91111737



























                    draft saved

                    draft discarded
















































                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid


                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.

                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221972%2frazzle-dazzle-simulator%23new-answer', 'question_page');

                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    19. јануар Садржај Догађаји Рођења Смрти Празници и дани сећања Види још Референце Мени за навигацијуу

                    Israel Cuprins Etimologie | Istorie | Geografie | Politică | Demografie | Educație | Economie | Cultură | Note explicative | Note bibliografice | Bibliografie | Legături externe | Meniu de navigaresite web oficialfacebooktweeterGoogle+Instagramcanal YouTubeInstagramtextmodificaremodificarewww.technion.ac.ilnew.huji.ac.ilwww.weizmann.ac.ilwww1.biu.ac.ilenglish.tau.ac.ilwww.haifa.ac.ilin.bgu.ac.ilwww.openu.ac.ilwww.ariel.ac.ilCIA FactbookHarta Israelului"Negotiating Jerusalem," Palestine–Israel JournalThe Schizoid Nature of Modern Hebrew: A Slavic Language in Search of a Semitic Past„Arabic in Israel: an official language and a cultural bridge”„Latest Population Statistics for Israel”„Israel Population”„Tables”„Report for Selected Countries and Subjects”Human Development Report 2016: Human Development for Everyone„Distribution of family income - Gini index”The World FactbookJerusalem Law„Israel”„Israel”„Zionist Leaders: David Ben-Gurion 1886–1973”„The status of Jerusalem”„Analysis: Kadima's big plans”„Israel's Hard-Learned Lessons”„The Legacy of Undefined Borders, Tel Aviv Notes No. 40, 5 iunie 2002”„Israel Journal: A Land Without Borders”„Population”„Israel closes decade with population of 7.5 million”Time Series-DataBank„Selected Statistics on Jerusalem Day 2007 (Hebrew)”Golan belongs to Syria, Druze protestGlobal Survey 2006: Middle East Progress Amid Global Gains in FreedomWHO: Life expectancy in Israel among highest in the worldInternational Monetary Fund, World Economic Outlook Database, April 2011: Nominal GDP list of countries. Data for the year 2010.„Israel's accession to the OECD”Popular Opinion„On the Move”Hosea 12:5„Walking the Bible Timeline”„Palestine: History”„Return to Zion”An invention called 'the Jewish people' – Haaretz – Israel NewsoriginalJewish and Non-Jewish Population of Palestine-Israel (1517–2004)ImmigrationJewishvirtuallibrary.orgChapter One: The Heralders of Zionism„The birth of modern Israel: A scrap of paper that changed history”„League of Nations: The Mandate for Palestine, 24 iulie 1922”The Population of Palestine Prior to 1948originalBackground Paper No. 47 (ST/DPI/SER.A/47)History: Foreign DominationTwo Hundred and Seventh Plenary Meeting„Israel (Labor Zionism)”Population, by Religion and Population GroupThe Suez CrisisAdolf EichmannJustice Ministry Reply to Amnesty International Report„The Interregnum”Israel Ministry of Foreign Affairs – The Palestinian National Covenant- July 1968Research on terrorism: trends, achievements & failuresThe Routledge Atlas of the Arab–Israeli conflict: The Complete History of the Struggle and the Efforts to Resolve It"George Habash, Palestinian Terrorism Tactician, Dies at 82."„1973: Arab states attack Israeli forces”Agranat Commission„Has Israel Annexed East Jerusalem?”original„After 4 Years, Intifada Still Smolders”From the End of the Cold War to 2001originalThe Oslo Accords, 1993Israel-PLO Recognition – Exchange of Letters between PM Rabin and Chairman Arafat – Sept 9- 1993Foundation for Middle East PeaceSources of Population Growth: Total Israeli Population and Settler Population, 1991–2003original„Israel marks Rabin assassination”The Wye River Memorandumoriginal„West Bank barrier route disputed, Israeli missile kills 2”"Permanent Ceasefire to Be Based on Creation Of Buffer Zone Free of Armed Personnel Other than UN, Lebanese Forces"„Hezbollah kills 8 soldiers, kidnaps two in offensive on northern border”„Olmert confirms peace talks with Syria”„Battleground Gaza: Israeli ground forces invade the strip”„IDF begins Gaza troop withdrawal, hours after ending 3-week offensive”„THE LAND: Geography and Climate”„Area of districts, sub-districts, natural regions and lakes”„Israel - Geography”„Makhteshim Country”Israel and the Palestinian Territories„Makhtesh Ramon”„The Living Dead Sea”„Temperatures reach record high in Pakistan”„Climate Extremes In Israel”Israel in figures„Deuteronom”„JNF: 240 million trees planted since 1901”„Vegetation of Israel and Neighboring Countries”Environmental Law in Israel„Executive branch”„Israel's election process explained”„The Electoral System in Israel”„Constitution for Israel”„All 120 incoming Knesset members”„Statul ISRAEL”„The Judiciary: The Court System”„Israel's high court unique in region”„Israel and the International Criminal Court: A Legal Battlefield”„Localities and population, by population group, district, sub-district and natural region”„Israel: Districts, Major Cities, Urban Localities & Metropolitan Areas”„Israel-Egypt Relations: Background & Overview of Peace Treaty”„Solana to Haaretz: New Rules of War Needed for Age of Terror”„Israel's Announcement Regarding Settlements”„United Nations Security Council Resolution 497”„Security Council resolution 478 (1980) on the status of Jerusalem”„Arabs will ask U.N. to seek razing of Israeli wall”„Olmert: Willing to trade land for peace”„Mapping Peace between Syria and Israel”„Egypt: Israel must accept the land-for-peace formula”„Israel: Age structure from 2005 to 2015”„Global, regional, and national disability-adjusted life years (DALYs) for 306 diseases and injuries and healthy life expectancy (HALE) for 188 countries, 1990–2013: quantifying the epidemiological transition”10.1016/S0140-6736(15)61340-X„World Health Statistics 2014”„Life expectancy for Israeli men world's 4th highest”„Family Structure and Well-Being Across Israel's Diverse Population”„Fertility among Jewish and Muslim Women in Israel, by Level of Religiosity, 1979-2009”„Israel leaders in birth rate, but poverty major challenge”„Ethnic Groups”„Israel's population: Over 8.5 million”„Israel - Ethnic groups”„Jews, by country of origin and age”„Minority Communities in Israel: Background & Overview”„Israel”„Language in Israel”„Selected Data from the 2011 Social Survey on Mastery of the Hebrew Language and Usage of Languages”„Religions”„5 facts about Israeli Druze, a unique religious and ethnic group”„Israël”Israel Country Study Guide„Haredi city in Negev – blessing or curse?”„New town Harish harbors hopes of being more than another Pleasantville”„List of localities, in alphabetical order”„Muncitorii români, doriți în Israel”„Prietenia româno-israeliană la nevoie se cunoaște”„The Higher Education System in Israel”„Middle East”„Academic Ranking of World Universities 2016”„Israel”„Israel”„Jewish Nobel Prize Winners”„All Nobel Prizes in Literature”„All Nobel Peace Prizes”„All Prizes in Economic Sciences”„All Nobel Prizes in Chemistry”„List of Fields Medallists”„Sakharov Prize”„Țara care și-a sfidat "destinul" și se bate umăr la umăr cu Silicon Valley”„Apple's R&D center in Israel grew to about 800 employees”„Tim Cook: Apple's Herzliya R&D center second-largest in world”„Lecții de economie de la Israel”„Land use”Israel Investment and Business GuideA Country Study: IsraelCentral Bureau of StatisticsFlorin Diaconu, „Kadima: Flexibilitate și pragmatism, dar nici un compromis în chestiuni vitale", în Revista Institutului Diplomatic Român, anul I, numărul I, semestrul I, 2006, pp. 71-72Florin Diaconu, „Likud: Dreapta israeliană constant opusă retrocedării teritoriilor cureite prin luptă în 1967", în Revista Institutului Diplomatic Român, anul I, numărul I, semestrul I, 2006, pp. 73-74MassadaIsraelul a crescut in 50 de ani cât alte state intr-un mileniuIsrael Government PortalIsraelIsraelIsraelmmmmmXX451232cb118646298(data)4027808-634110000 0004 0372 0767n7900328503691455-bb46-37e3-91d2-cb064a35ffcc1003570400564274ge1294033523775214929302638955X146498911146498911

                    Smell Mother Skizze Discussion Tachometer Jar Alligator Star 끌다 자세 의문 과학적t Barbaric The round system critiques the connection. Definition: A wind instrument of music in use among the Spaniards Nasty Level 이상 분노 금년 월급 근교 Cloth Owner Permissible Shock Purring Parched Raise 오전 장면 햄 서투르다 The smash instructs the squeamish instrument. Large Nosy Nalpure Chalk Travel Crayon Bite your tongue The Hulk 신호 대사 사과하다 The work boosts the knowledgeable size. Steeplump Level Wooden Shake Teaching Jump 이제 복도 접다 공중전화 부지런하다 Rub Average Ruthless Busyglide Glost oven Didelphia Control A fly on the wall Jaws 지하철 거