class Calculation():
def __init__(self, calculation):
#init
self.calculation = calculation
Вы сделали класс `Вычисления`, чтобы проводить вычисления, пока проводятся вычисления вычислений над вычислениями, которые вычисляются как аргумент вычислений для вычисления состояния вычислений.
Вы бы хотя бы в предметную область нас тут погрузили хоть немножечку. Не понятно же ничерта. Обычно класс и инстанс можно называть одинаково, за исключением первой буквы, но вы тут в аргументы что-то передаёте и то нифига не понятно. Делайте докстринги.
Используйте тайпхинтинг, это позволяет не только статичесий анализ кода делать и избегать лишних ошибок, но, к тому же, это мощный инструмент документирования кода, неотрывно связанный с самим кодом. Это значит, что документация не отстанет от кода, а, напротив, будет за счет формализма и машиночитаемости помогать IDE помогать нам писать код. К тому же ряд вопросов отпадёт у тех, кто пытается читать код. Не нужно гадать на кофейной гуще что есть что.
Нужно помнить, что исходный код пишется не для компьютеров, а для людей. Должно ыть удобно код писать, но любой код пишут только один раз, а вот читают его каждй раз, когда нужно разобраться что пошло не так, каждый раз, когда нужно расширить функциональность, исправить ошибки, изменить логику... Читабельность для кода гораздо важнее писабельности.
У вас в коде полно "магических" констант. Именуйте их и выносите в начало модуля или, хотя бы, указывайте в инлайн-комментах единицы измерения для ясности. Не пренебрегайте свойствами, их можно документировать .
Вы тут много работаете с календарными периодами. перебираете их с шагом... Это хорошее место для выделения функциональности в отдельную библиотеку, в отдельный тип данных. Возможно писать свой велосипед даже не придётся, ведь найдётся много готовых качественных протестированных библиотека для этих целей. Их будет легко и понятно инициализировать, у них будет простой, понятный и универсальный АПИ, они будут однотипно использованы в разных мемтах проекта, не придётся смешивать в одном контесте кучу переменных для обеспечения двух имплементаций одной и той же функциональности.
Количество кода резко убавится, универсальный код будет вынесен в отдельный модуль и будет отдельно и полноценно протестирован, а в бизнес логике вы будете коротко и лаконично работать с абстракцией - понятной и простой.
Вот тут у вас, очевидно, можно написать проще и без лишних повторений, провоцирующих ошибки:
if i % self.inflation_indexation_period==0 and i != 0:
if i not in range(0, self.inflation_indexation_period):
init_indexation_inflation *= self.sales_init.inflation_indexation
else:
init_indexation_inflation = 1
inflation_indexations.append(round(init_indexation_inflation, 5))
else:
inflation_indexations.append(round(init_indexation_inflation, 5))
Лучше так:
if i % self.inflation_indexation_period==0 and i != 0:
if i in range(self.inflation_indexation_period):
init_indexation_inflation = 1
else:
init_indexation_inflation *= self.sales_init.inflation_indexation
inflation_indexations.append(round(init_indexation_inflation, 5))
Функция get_inflation_indexations у вас имеет опасный побочный эффект. Она имеет префикс get_ но модифицирует контекст объекта. Это кэширование? Чем обусловлено такое поведение? Если такое делается "на всякий случай". то это плохая практика неявного внедрения побочного эффекта. Если нарочно, то такое надо документировать и корректно называть и описывать метод в докстринге.
Опять же, get_inflation_indexations и get_value_indexations очень похожи по коду. Это повод вынести такую логику в отельную функцию, она будет проще и её будет проще тестировать!
А у вас эти функции отличаются именами атрибутов внутри и магическими константами, которые в коде делать не хорошо, тем более без пояснений, тем более в кусках такого похожего кода.
Перестаньте использовать i в качестве переменной для итерирования нетривиальных сущностей, отличных от протсого счетчика. i - это индекс. Используйте человеко-понятное название переменной для этого!
Используйте декоратор итераторов enumerate. Это сделает код более прозрачным и читабельным, чем код с параллельными счетчиками. Увидев enumerate читатель кода сразу поймёт, что это простой счетчик итерируемых сущностей, что не нужно ожидать скачков этого счетчика и каких-то сложных корреляций.
А вот здесь вообще всё плохо:
count = 0
revenue_list = []
for i in total_price:
revenue = i*total_value[count]
revenue_list.append(revenue)
count+=1
count - это "количество", а вы его используете как "индекс" и никак иначе!
i - это индекс, а вы туда суёте фактически цену!
У вас total_price и total_value параллельные одноразмерные списки, их нужно состегнуть с помощью zip и пронумеровать с помощью enumerate (если надо, а здесь не надо!).
Весь этот кусок понятнее, проще, короче и более питоничнее записать в такой форме:
revenue_list = [price * value for price, value in zip(total_prices, total_values)]
Итого вся вот эта громоздкая плохо читабельная функция:
def get_revenue(self):
'''Получить итоговую выручку'''
total_price = []
for i in self.get_inflation_indexations():
price = self.sales_init.price*i
total_price.append(price)
total_value = []
for i in self.get_value_indexations():
value = self.sales_init.sales_volume*i
total_value.append(value)
count = 0
revenue_list = []
for i in total_price:
revenue = i*total_value[count]
revenue_list.append(revenue)
count+=1
return revenue_list
Легко и читабельно для питониста заменяется на вот такую:
def get_revenue(self):
'''Получить итоговую выручку'''
indexations = self.get_inflation_indexations()
init_price = self.sales_init.price
total_prices = [init_price * x for x in indexations]
init_volume = self.sales_init.volume
total_values = [init_volume * x for x in indexations]
return [price * value for price, value in zip(total_prices, total_values)]
И везде не стоит использовать параллельные счетчики, используйте итераторы, распаковку, зипы, енумервторы и функциональный стиль, ведь он сокращает код и делает его проще.
Что это за ерунда:
def get_interest_expenses(self):
'''процентные расходы'''
interest_expenses_list = []
return interest_expenses_list
Зачем много раз считать одно и то же целиком, чтобы взять только очередной кусочек из всего посчитанного?!
Это вообще бред. Учитесь основам алгоритмизации и не надо программировать на питоне как не на питоне.
Вот такое вообще жесть:
self.get_revenue()[count]
Отчего не сохранить в промежуточную переменную?!
В общем, всё плохо.
Если у вас есть функция, вычисляющая какой-то список, то зачем её вычислять каждый раз, когда вам нужен только один очередной элеиент этого списка, а вы перебираете его целиком?!
И так много раз везде!
Тут не архитектура хромает, тут основы алгоритмизации плачут. Тренируйтесь на кошках, сударь, больше решайте алгоритмических задачек. Структурируйте, декомпозируйте.
Удачи.
Простой
Средний
Сложный
Средний