Code Review Asked by Lucas Melo on January 4, 2022
I’m new to programming and this is the first thing I’m doing on my own. I would appreciate it if you could point me ways to optimize this RPG dice roller code in Python!
import random
def dice_reader():
##this should read messages like '2 d 6 + 3, 5 d 20 + 2 or just 3 d 8'##
message = input('>')
reader = message.split(' ')
times = reader[0]
sides = reader[2]
output_with_modifier = []
result = []
if len(reader) == 5:
modifier = reader[4]
else:
modifier = 0
for output in range(int(times)):
output = random.randint(1, int(sides))
result.append(output)
output_with_modifier = [(int(x) + int(modifier)) for x in result]
print(f' Dice rolls: {tuple(result)}')
if modifier != 0:
print(f' With modifier: {tuple(output_with_modifier)}')
end = False
while end == False:
dice_reader()
end_message = input('Again? ')
if end_message.lower() == 'no':
end = True
else:
pass
It's clear that there's an implied structure to this input:
message = input('>')
reader = message.split(' ')
requiring between four and five tokens. I have no idea what those are, and neither does your user. Replace '>'
with an actual description of what's expected here.
times = reader[0]
sides = reader[2]
can be
times, _, sides = reader[:3]
though the discarded second item is suspicious. You do need to show what this is, and it probably shouldn't be there.
This:
for output in range(int(times)):
output_with_modifier = [(int(x) + int(modifier)) for x in result]
does not make sense. If you ask for 100 times, output_with_modifier
will be calculated 100 times and thrown away 99 of them. Only the last value will be kept. You probably want to de-indent that last assignment so that it happens outside of the loop.
end = False
while end == False:
dice_reader()
end_message = input('Again? ')
if end_message.lower() == 'no':
end = True
else:
pass
First, delete that else; pass
- it isn't doing anything. Also, end == False
should be not end
; but you shouldn't be using a termination variable at all. If you find a no
, simply break
.
Some of this may challenge a beginner, but I like to think that CodeReview is for "aspiring advanced programmers". I've tried to comment it extensively, but feel free to ask questions in the comments.
import re
from random import randint
from re import Pattern
from typing import ClassVar, Iterable
class Dice:
"""
One specification for dice rolls in Dungeons & Dragons-like format.
"""
def __init__(self, times: int, sides: int, modifier: int = 0):
if times < 1:
raise ValueError(f'times={times} is not a positive integer')
if sides < 1:
raise ValueError(f'sides={sides} is not a positive integer')
self.times, self.sides, self.modifier = times, sides, modifier
# This is a class variable (basically a "static") that only has one copy
# for the entire class type, rather than a copy for every class instance
# It is a regular expression pattern that will allow us to parse user
# input.
INPUT_PAT: ClassVar[Pattern] = re.compile(
# From the start, maybe some whitespace, then a group named "times"
# that contains one or more digits
r'^s*(?P<times>d+)'
# Maybe some whitespace, then the letter "d"
r's*d'
# Maybe some whitespace, then a group named "sides" that contains one
# or more digits
r's*(?P<sides>d+)'
# The beginning of a group that we do not store.
r'(?:'
# Maybe some whitespace, then a "+" character
r's*+'
# Maybe some whitespace, then a group named "modifier" that
# contains one or more digits
r's*(?P<modifier>d+)'
# End of the group that we do not store; mark it optional
r')?'
# Maybe some whitespace, then the end.
r's*$',
# We might use "d" or "D"
re.IGNORECASE
)
# This can only be called on the class type, not a class instance. It
# returns a new class instance, so it acts as a secondary constructor.
@classmethod
def parse(cls, message: str) -> 'Rolls':
match = cls.INPUT_PAT.match(message)
if match is None:
raise ValueError(f'Invalid dice specification string "{message}"')
# Make a new instance of this class based on the matched regular
# expression.
return cls(
int(match['times']),
int(match['sides']),
# If there was no modifier specified, pass 0.
0 if match['modifier'] is None else int(match['modifier']),
)
@classmethod
def from_stdin(cls) -> 'Rolls':
"""
Parse and return a new Rolls instance from stdin.
"""
while True:
try:
message = input(
'Enter your dice specification, of the formn'
'<times>d<sides> [+ modifier], e.g. 3d6 or 4d12 + 1:n'
)
return cls.parse(message)
except ValueError as v:
print(v)
print('Please try again.')
def roll(self, with_modifier: bool = False) -> Iterable[int]:
"""
Return a generator of rolls. This is "lazy" and will only execute the
rolls that are consumed by the caller, because it returns a generator
(not a list or a tuple).
"""
mod = self.modifier if with_modifier else 0
return (
randint(1, self.sides) + mod
for _ in range(self.times)
)
def print_roll(self):
print(
'Dice rolls:',
', '.join(str(x) for x in self.roll()),
)
if self.modifier != 0:
print(
'With modifier:',
', '.join(str(x) for x in self.roll(with_modifier=True)),
)
def test():
"""
This is an automated test method that does some sanity checks on the Dice
implementation.
"""
d = Dice.parse('3 d 6')
assert d.times == 3
assert d.sides == 6
assert d.modifier == 0
d = Dice.parse('3D6 + 2')
assert d.times == 3
assert d.sides == 6
assert d.modifier == 2
try:
Dice.parse('nonsense')
raise AssertionError()
except ValueError as v:
assert str(v) == 'Invalid dice specification string "nonsense"'
try:
Dice.parse('-2d5')
raise AssertionError()
except ValueError as v:
assert str(v) == 'Invalid dice specification string "-2d5"'
try:
Dice.parse('0d6')
raise AssertionError()
except ValueError as v:
assert str(v) == "times=0 is not a positive integer"
d = Dice.parse('100 d 12+3')
n = 0
for x in d.roll(True):
assert 4 <= x <= 15
n += 1
assert n == 100
def main():
test()
dice = Dice.from_stdin()
dice.print_roll()
if __name__ == '__main__':
main()
Answered by Reinderien on January 4, 2022
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP