Платежи и обязательства


  • Получить имена от names.txt
  • Вам сделок с transactions.txt
  • Выходные данные программа должна показать, кто должен, что вот так:

Alice pays $xx.xx to David.
Bob pays $xx.xx to Claire.

И так далее.

names.txt

Alice
Bob
Claire
David

transactions.txt

Bob paid $55.90 for petrol.
Bob paid $70 for house-cleaning.
Bob paid $85.60 for lunch.
Claire paid $100.10 for phone bill.
Claire paid $103.45 for dinner.
Claire paid $30.80 for snacks.
David paid $170.80 for groceries.
David paid $33.40 for breakfast.
David paid $63.50 for utilities.

Мой код работает и получает правильный результат. Я просто интересно, если есть более рациональный способ.

Этот код создает объект оплаты, добавляет людей, а затем добавляет платежи, после чего получает обязательства.

import re
from decimal import Decimal

def filer(file):
    f = open(file, 'r')
    lines = []
    for line in f.readlines():
        lines.append(line.strip())
    return lines

def parse_lines(lines, **kwargs):
    compilers = kwargs.get('keys', ['money', 'name', 'bill'])
    parses = []
    for line in lines:
        l1 = Parser(line, keys=compilers)
        parses.append(l1.get_dict())
    return parses


class Parser(object):
    patterns    = {'name_pattern':'^([\w\-]+)','money_pattern':'(\d+.?\d*)','bill_pattern':"[\w'-]+\.$"}
    keys        = ['money', 'name', 'bill']

    def __init__(self, line, **kwargs):
        if 'patterns' in kwargs:
            self.patterns.update(kwargs.get('patterns'))
            del kwargs['patterns']

        self.__dict__.update(kwargs)

        for key in self.keys:
            key_str = key+'_pattern'
            parser  = re.compile(self.patterns[key_str])
            value   = parser.search(line)
            setattr(self, key, value.group(0))

    def get_dict(self):
        rtn = {}
        for key in self.keys:
            rtn[key] = getattr(self, key)
        return rtn

class Payments(object):

    def __init__(self, **kwargs):
        self.people         = {}
        self.formatter  = "{from} ​pays​ {currency}{amount} ​to​ {to}."
        self.currency   = '$'
        self.__dict__.update(kwargs)

    def add_person(self, person):
        self.people[str(person.name)] = person

    def add_payment(self, payment):
        person = self.people[payment['name']]
        person.add_to(payment)

    def get_liabilities(self):
        liabilities     = []
        for name, person in self.people.items():
            person.set_share(len(self.people.items()))

        for name, person in self.people.items():
            """gets share & sets it against each person in the group"""
            person.set_liability(self.people, self.currency)
            liabilities += person.liabilities

        return liabilities

    def print_liabilities(self):
        liabilities = []
        for liability in self.get_liabilities():
            liabilities.append(self.formatter.format(**liability))
        return liabilities

class Person(object):
    def __init__(self, **kwargs):
        self.name   = ""
        self.items  = []
        self.paid   = 0
        self.share  = 0
        self.liabilities = []
        self.__dict__.update(kwargs)

    def add_to(self, payment):
        """ payment must have keys: 'name', 'money'"""
        if payment['name'] == self.name:
            self.items.append(payment)

    def _get_paid(self):
        balance = 0
        for item in self.items:
            amount  = Decimal(item['money'],2)
            balance += amount
        return Decimal(balance, 2)

    def set_share(self, total_people):
        """total amount paid is divided by the number of 
        people to get the liabilities of each person"""
        self.paid   = self._get_paid()
        self.share  = round(self.paid/Decimal(total_people), 2)

    def _set_extras(self, liability, keys):
        """sets extra properties"""
        for key in keys:
            if hasattr(self, key):
                liab_dict[key] = getattr(self, key)

    def set_liability(self, people, currency):
        """if owed is share is greater than owed, name needs to pay this"""
        for name, person in people.items():
            if self.name != name:
                """if owed is greater than what I have paid then add to liabilities"""
                diff = self.share - person.share
                if diff < 0:
                    liab_dict = {'from':self.name,'to': name,'amount':-diff, 'currency':currency }
                    self.liabilities.append(liab_dict)

class PaymentBuilder(object):
    people  = []
    def __init__(self, **kwargs):
        transactions    = kwargs.get('transaction_file','./transactions.txt')
        names           = kwargs.get('names_file','./names.txt')
        self.payments   = Payments()
        self.trans      = parse_lines(filer(transactions))
        self.names      = filer(names)

        for name in self.names:
            self.payments.add_person(Person(name=name))
        for tran in self.trans:
            self.payments.add_payment(tran)

    def liabilities(self):
        return [ liability for liability in self.payments.print_liabilities()]

Для запуска просто init PageBuilder тогда вам обязательств. Пожалуйста, обязательно размещайте names.txt и transactions.txt в корневой папке, иначе не пройдет мест.

Для конкретного места использования файл:

PaymentBuilder(transaction_file='/var/location/filename.txt')

payments = PaymentBuilder()
for line in payments.liabilities():
    print(line)

Выход:

Claire ​pays​ $8.33 ​to​ David.
Bob ​pays​ $5.71 ​to​ Claire.
Bob ​pays​ $14.04 ​to​ David.
Alice ​pays​ $58.59 ​to​ Claire.
Alice ​pays​ $66.92 ​to​ David.
Alice ​pays​ $52.88 ​to​ Bob.


218
9
задан 13 февраля 2018 в 03:02 Источник Поделиться
Комментарии
2 ответа

Низко висящие фрукты

Вам надо почитать PEP8 , чтобы сделать ваш код выглядеть как код Python для окружающих. Вас есть несколько нарушений:


  • расстояние по операторам (+, ,и = в основном);

  • пустые строки вокруг кода верхнего уровня;

  • различия между замечания и комментарии.

Также **kwargs вещь... я не понимаю, почему вы используете его в большинстве своих функций и конструкторов. Вам не нужно переменное число именованных аргументов, все, что вам нужно-это значение по умолчанию для некоторые из них: использовать более идиоматические

def __init__(self, transactions_file='./transactions.txt', names_file='./names.txt'):

(для PaymentBuilder) вместо.

Вы должны также использовать with заявление для управления файлами, а не вручную их закрывать, что вы забыли сделать в любом случае...

Наконец, Person._set_extras метод никогда не вызывается и могут быть безопасно удалены (тем более, что он использует неопределенную переменную).

Перестаньте писать классы

Два занятия только __init__ и единственный метод. Они должны быть, а не функции, так как здесь нет какого-то государства, что мы должны манипулировать.

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

Вся логика PaymentBuilder.liabilities() может быть выражен как следует, из вашего кода:


  • Для каждого имени в файле инициализации dict записи в пустую list для хранения будущих сделок

  • Для каждой транзакции в файл, хранить суммы потратить на правильное название в вышеупомянутом словаре

  • Для каждой записи в словаре, вычислить долю проводимого человеком

  • Для каждой пары человек, вычислить разницу в потраченных акций и, если первый чем-то обязаны на второй, печати, информации.

Каждый из этих шагов может рассматриваться как дикт понимания или перебираем содержимое словаря.

Использовать генераторы

Вместо того, чтобы строить списки и добавления к ним для того, чтобы построить другие списки, добавляя в них... вы можете уменьшить объем памяти с помощью генераторов.

Это позволит вам обрабатывать файлы построчно, например, без того, чтобы читать их заранее. Или печатать обязательств по одному без того, чтобы вычислить их все заранее, или.

Предлагаемые улучшения

import re
from decimal import Decimal

def filer(filename):
with open(filename, 'r') as f:
yield from map(str.strip, f)

def parse_line(line, patterns):
return {
key: parser.search(line).group(0)
for key, parser in patterns.items()
}

def parse_lines(lines, patterns=None):
if patterns is None:
patterns = {
'name': re.compile('^([\w\-]+)'),
'money': re.compile('(\d+.?\d*)'),
'bill': re.compile("[\w'-]+\.$"),
}
yield from (parse_line(line, patterns) for line in lines)

def liabilities(transactions_file='./transactions.txt', names_file='./names.txt', currency='$'):
persons = {name: [] for name in filer(names_file)}

for transaction in parse_lines(filer(transactions_file)):
persons[transaction['name']].append(transaction['money'])

total_people = Decimal(len(persons))
shares = {
name: round(sum(Decimal(p) for p in paid) / total_people, 2)
for name, paid in persons.items()
}

for name, share in shares.items():
for other, other_share in shares.items():
if name == other:
continue
difference = share - other_share
if difference < 0:
yield '{debitor} pays {currency}{amount} to {creditor}.'.format(
currency=currency,
amount=-difference,
debitor=name, creditor=other)

if __name__ == '__main__':
for line in liabilities():
print(line)

8
ответ дан 13 февраля 2018 в 09:02 Источник Поделиться

Дизайн

Я повторю совет от @MathiasEttinger перестать писать классы. Я думаю, что ваше решение более-инженерии. Однако, в то же время, ваша конструкция является хрупкой и склонной к злоупотреблениям. Некоторые замечания о Вашей конструкции:


  • Вы полагаетесь на определенную последовательность мутаций, которые необходимо выполнить, в частности, в Payments.get_liabilities():


    def get_liabilities(self):
    liabilities = []
    for name, person in self.people.items():
    person.set_share(len(self.people.items()))

    for name, person in self.people.items():
    """gets share & sets it against each person in the group"""
    person.set_liability(self.people, self.currency)
    liabilities += person.liabilities

    return liabilities


    Так что, прежде чем get_liabilities() называется, people должны уже были свои расходы добавил. Затем в течение person.set_liability()вас ожидают каждого другого человека .share уже были вычислены. Что правильного использования Person класс не вытекает из каких-то документации Person класс.

    (Кстати, len(self.people) хватило бы в выдержке выше; len(self.people.items()) это перебор.)


  • Расчет на самом деле включает в себя вложенный цикл for, но как это работает затемняется из-за того, что внешний контур в Payments.get_liabilities()и внутренний цикл выполняется в Person.set_liability().

  • Payments.print_liabilities() это вводит в заблуждение. Он ничего не печатает!

  • Мультивалютная поддержка не эффективна. Это в основном упражнения в параметр-прохождения, после установки жестко self.currency = '$' в конструкторе Payments класс. Если бы ты сослался на $ символ от парсинга transactions.txt и прошли ее вместе, тогда это будет полезно. В любом случае, форматирование валюты может быть сложным. Некоторые валютные символы, например, традиционно ставится после числа. В резюме, это упрощенное попытки не стоит. Вы бы лучше жесткого кодирования $ знак в formatter строку.

  • Person._set_extras() не используется (и если их использовать, будет признаком плохого класса дизайн).

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

Стиль


  • Большую часть кода можно было бы сократить, написав выделения вместо for петли. Например, filer(), Parser.get_dict(), Payments.print_liabilities()и Person._get_paid() может быть один-вкладыши (или почти так).

    С другой стороны, это совершенно избыточные понимания список:


    def liabilities(self):
    return [ liability for liability in self.payments.print_liabilities()]


  • Не злоупотреблять **kwargs. Ваш PaymentBuilder конструктор должен принимать два параметра (со значениями по умолчанию). Ваш parse_lines() функция должна принимать один список. Что касается вашего Parser конструктор, я не знаю, какую магию он пытается применить. (Во всяком случае, как я уже упоминал выше, ваш Parser должен быть всего лишь один жестко заданный регулярным выражением.)

  • Использовать r'raw strings' для регулярного выражения, содержащие обратные слеши, чтобы избежать возможного символы интерпретируются неверно.

  • Писать """docstrings""" для всех ваших методов. Но не писать комментарии там, где они на самом деле # code comments.

Предлагаемое решение

Я бы просто написать две функции, плюс немного кода клей для вызова функций и распечатать результаты. Обратите внимание, что все десятичные округление происходит в праматерии.

import re
from decimal import Decimal

def parse_expenses(names_file, transactions_file):
"""
Parse a file containing transactions to obtain a dict listing the total
expenses incurred by each person listed in the names file.

names_file should be a path to a text file, containing one name per line.
transactions_file should be a path to a text file, with each line of the
form "John paid $1.23 for blah".
"""
regex = re.compile(r'^(?P<name>[\w-]+).*?\$(?P<amount>\d+.?\d*)')
with open(names_file) as f:
expenses = {line.strip(): Decimal('0') for line in f}
with open(transactions_file) as f:
for line in f:
match = regex.match(line)
if not match or match.group('name') not in expenses:
raise ValueError('Bad transaction: "{0}"'.format(line.strip()))
expenses[match.group('name')] += Decimal(match.group('amount'))
return expenses

def equalization_payments(expenses):
"""
Given a dictionary that maps names to expenses, generate
(payer, payee, amount) tuples that would result in the expenses being
equalized.
"""
n = len(expenses)
for debtor, debt in expenses.items():
for creditor, credit in expenses.items():
diff = credit - debt
if diff > 0:
yield debtor, creditor, diff / n

if __name__ == '__main__':
expenses = parse_expenses('names.txt', 'transactions.txt')
for payer, payee, amount in equalization_payments(expenses):
print('{payer} pays ${amount:0.2f} to {payee}.'.format(
payer=payer, payee=payee, amount=amount
))

6
ответ дан 14 февраля 2018 в 12:02 Источник Поделиться