Ищите отзывы о моем проекте программы

Question

Я знаю, что SO - это вопрос, но в целом цель состоит в том, чтобы помочь людям научиться, поэтому я решил, что попробую использовать свой код, чтобы поделиться своим кодом и запросить отзывы об этом.

---

Я ищу для создания программы, которая будет полагаться на случайные числа, а именно кости. Они будут представлены в виде «2D6», «4D10 + 3», «2D2 + 3D3» и т. Д. И т. Д. Таким образом, я решил создать модуль роликов с кубиками, который мог бы принимать ввод, как в этой форме.

Он отлично работает для того, что необходимо, но имеет ошибку для вещей, которые, вероятно, не понадобятся (должно быть объяснено докштрина в начале файла). Меня интересует то, что люди думают о моем коде, и если кто-то может увидеть способы его улучшить.

Это все еще WIP, и я еще не начал модульные тесты.

Link to code

#!/usr/bin/env python3 
""" 
Created by Teifion Jordan 
http://woarl.com 

Notes: The roller does not correctly apply * and/signs: 
A + B * C is worked out as (A + B) * C, not A + (B * C) as would be correct 
""" 

import random 
import re 
import math 

class Roller_dict (object): 
    """A 'dictionary' that stores rollers, if it's not got that roller it'll make a new one""" 
    def __init__(self, generator=random.randint): 
     super(Roller_dict, self).__init__() 
     self.rollers = {} 

     # Generator is used to supply a "rigged" random function for testing purposes 
     self.generator = generator 

    def __call__(self, constructor): 
     constructor = constructor.replace(" ", "") 
     if constructor not in self.rollers: 
      self.rollers[constructor] = Roller(constructor, self.generator) 

     return self.rollers[constructor]() 

# Regular expressions used by the Roller class 
# Compiled here to save time if we need to make lots of Roller objects 
pattern_split  = re.compile(r"(\+|-|\*|/)") 
pattern_constant = re.compile(r"([0-9]*)") 
pattern_die   = re.compile(r"([0-9]*)[Dd]([0-9]*)") 
pattern_sign  = re.compile(r"^(\+|-|\*|/)") 

class Roller (object): 
    def __call__(self): 
     return self.roll() 

    def __init__(self, constructor, generator=random.randint): 
     super(Roller, self).__init__() 
     self.items = [] 
     self.rebuild(constructor) 
     self.generator = generator 

    def rebuild(self, constructor): 
     """Builds the Roller from a new constructor string""" 
     # First we need to split it up 
     c = pattern_split.split(constructor.replace(" ", "")) 

     # Check for exceptions 
     if len(c) == 0: 
      raise Exception('String "%s" did not produce any splits' % constructor) 

     # Stitch signs back into their sections 
     parts = [] 
     last_p = "" 
     for p in c: 
      if p in "+-*/": 
       last_p = p 
       continue 

      if last_p != "": 
       p = "%s%s" % (last_p, p) 
       last_p = "" 

      parts.append(p) 

     # We have the parts, now we need to evaluate them into items 
     for p in parts: 
      # Look for a sign, default to positive 
      sign = pattern_sign.search(p) 
      if sign == None: sign = "+" 
      else: sign = sign.groups()[0] 

      # Strip out the sign, we're left with just the pure value 
      body = p.replace(sign, "") 

      # Now we find out what our main body is 

      # Die 
      value = pattern_die.search(body) 
      if value != None: 
       # Sign, Number, Sides 
       self.items.append(("die", sign, int(value.groups()[0]), int(value.groups()[1]))) 
       continue 

      # Constant 
      value = pattern_constant.search(body) 
      if value != None: 
       self.items.append(("constant", sign, int(value.groups()[0]))) 
       continue 

      # No matches 
      raise Exception('The part string "%s" had no matches' % body) 

    def roll(self): 
     """Rolls the die/dice and returns the result""" 
     result = 0 

     for i in self.items: 
      # Get value 
      if i[0] == "die":   value = self._derive_die(i[2], i[3]) 
      elif i[0] == "constant": value = self._derive_constant(i[2]) 
      else: raise Exception('No handler for item type "%s"' % i[0]) 

      # Apply sign 
      if i[1] == "+":  result += value 
      elif i[1] == "-": result -= value 
      elif i[1] == "*": result *= value 
      elif i[1] == "/": result /= value 

     return result 

    def _derive_die(self, number, sides): 
     result = 0 
     for n in range(0, number): 
      result += self.generator(0, sides) 

     return result 

    def _derive_constant(self, value): 
     return value 

# Useful for running the tests to make sure that it uses "random" numbers 
false_numbers = (int(math.cos(x)*5)+5 for x in range(0,1000)) 
def false_numbers_func(*args): 
    return false_numbers.next() 

# If it's main, run unit tests? 
if __name__ == '__main__': 
    r = Roller_dict(false_numbers_func) 

    print(r("2D6")) 
    print(r("2D6")) 
    print(r("2D6")) 

Answer 1

Поверхностно, есть PEP08; в частности, использование 4 пробелов для отступа vs с использованием символов табуляции.

Похоже, что здесь у вас много случайной сложности, но мне нужно будет пожевать его еще немного, чтобы понять. Похоже, достаточно простая идея, что он не должен прикладывать столько усилий, чтобы заглянуть, как кажется.

Answer 2

Я думаю, что ваш подход требует большой сложности: вы пытаетесь решить сложную проблему (разбор входных данных), в то же время вы решаете менее трудную задачу (совершая скатывание в кости). Это проще, если вы отделите проблемы.

Класс для броска кубика относительно прост в записи. Две вещи, которые я делаю, это вы: сопоставление знаков с операциями (использование карты означает отсутствие необходимости писать логику, плюс ее многократное использование), и позволяя объектам Roller объединяться в простой связанный список, чтобы вызов roll в начале списка свертывает все из них и суммирует результат.

import random 
R = random.Random() 

class Roller(object): 
    # map signs to operations 
    op = { "+" : lambda a,b: a+b, 
      "-" : lambda a,b: a-b, 
      "*" : lambda a,b: a*b, 
      "/" : lambda a,b: a/b } 

    def __init__(self, dice, sides, sign=None, modifier=0): 
     self.dice = dice 
     self.sides = sides 
     self.sign = sign 
     self.modifier = modifier 
     self.next_sign = None 
     self.next_roller = None 

    def roll(self): 
     self.dice_rolled = [R.randint(1, self.sides) for n in range(self.dice)] 
     result = sum(dice_rolled) 
     if self.sign: 
      result = self.op[self.sign](result, self.modifier) 
     if self.next_sign and self.next_roller: 
      result = self.op[self.next_sign](result, self.next_roller.roll()) 
     return result 

Относительно легко проверить это. Обратите внимание, что dice_rolled сохраняется как атрибут, чтобы вы могли легче записывать модульные тесты.

Следующий шаг - выяснить, как разобрать вход. Этот вид работ:

>>> p = """ 
(?P<next_sign>[-+*/])? 
(?P<dice>[\d]+) 
[\s]*D[\s]* 
(?P<sides>[\d]+) 
# trailing sign and modifier are optional, but if one is present both must be 
([\s]*(?P<sign>[-+/*])[\s]*(?P<modifier>[\d]+))?""" 
>>> r = re.compile(p, re.VERBOSE+re.IGNORECASE) 
>>> m=r.match('2 d 20 +1') 
>>> m.group('dice'), m.group('sides'), m.group('sign'), m.group('modifier') 
('2', '20', '+', '1') 
>>> r.findall('3D6*2-1D4+1*2D6-1') 
[('', '3', '6', '*2', '*', '2'), ('-', '1', '4', '+1', '+', '1'), ('*', '2', '6', '-1', '-', '1')] 

Там в лексической неоднозначности, что синтаксис позволяет - 2D6+1D4 получает разобрано как 2D6+1 с последующим несогласованной D4, и это не очевидно для меня, как исправить это в регулярном выражении. Возможно, это может быть исправлено с негативным прогнозом.

Во всяком случае, как только регулярное выражение фиксируется, осталось только обработать результаты r.findall, чтобы создать цепочку из Roller объектов. И сделайте этот метод класса, если вы действительно выкапываете инкапсуляцию.

Answer 3

pyparsing examples page включает в себя аналогичное dice expression parser and roller, в том числе этих тестов:

D5+2d6*3-5.5+4d6 
D5+2d6*3-5.5+4d6.takeHighest(3) 
2d6*3-5.5+4d6.minRoll(2).takeHighest(3) 

Первые 30 линий или так сценария содержит анализатор, остальное содержит вычислитель, в том числе отладки кода, показывающий валки прокатываемого ,

Я понимаю, что это скорее «серебряный тарелка», а не обратная связь с вашим опубликованным кодом. Одна вещь, общая с ответом Роберта Россни - четкое разделение синтаксического анализа против прокатки. Возможно, между этим и образцом Роберта вы можете собрать несколько лакомых кусочков для своего собственного кубика.