patternpythondjangoMinor
Bank account model implementation in Django
Viewed 0 times
djangoaccountbankimplementationmodel
Problem
There is a draft of my models.py. What can I do for code quality and readability increase?
```
from datetime import timedelta
from django.contrib.auth.models import User
from django.db import models
from django.utils import timezone
class Rule(models.Model):
start_date = models.DateField()
end_date = models.DateField()
deposit_percent = models.FloatField()
credit_percent = models.FloatField()
@staticmethod
def on_date(_date):
return Rule.objects.get(start_date__lte=_date, end_date__gte=_date)
class BankAccount(models.Model):
LEGAL = 1
NATURAL = 2
DEBET = 1
CREDIT = 2
owner_type = models.PositiveSmallIntegerField(choices=(
(LEGAL, 'Физическое лицо'),
(NATURAL, 'Юридическое лицо'),
))
method = models.PositiveSmallIntegerField(choices=(
(DEBET, 'Дебет'),
(CREDIT, 'Кредит'),
))
name = models.CharField(max_length=254)
owner = models.ForeignKey(User, related_name='w_accounts')
spectators = models.ManyToManyField(User, related_name='r_accounts')
when_opened = models.DateField()
@property
def balance(self):
assert self.method == self.DEBET
today = timezone.now()
rule = Rule.on_date(self.when_opened)
result = 0
current_date = self.when_opened
while current_date <= today:
result -= sum(x.total for x in self.cashing_set.on_date(current_date))
result -= sum(x.total for x in self.outcome_transfers.on_date(current_date))
result += sum(x.total for x in self.deposit_set.on_date(current_date))
result += sum(x.total for x in self.income_transfers.on_date(current_date))
result = 1 + rule.deposit_percent 0.01
current_date += timedelta(days=1)
return result
@property
def debt(self):
assert self.method == self.CREDIT
today = timezone.now()
rule = Rule.on_date(self.when_opened)
result = 0
```
from datetime import timedelta
from django.contrib.auth.models import User
from django.db import models
from django.utils import timezone
class Rule(models.Model):
start_date = models.DateField()
end_date = models.DateField()
deposit_percent = models.FloatField()
credit_percent = models.FloatField()
@staticmethod
def on_date(_date):
return Rule.objects.get(start_date__lte=_date, end_date__gte=_date)
class BankAccount(models.Model):
LEGAL = 1
NATURAL = 2
DEBET = 1
CREDIT = 2
owner_type = models.PositiveSmallIntegerField(choices=(
(LEGAL, 'Физическое лицо'),
(NATURAL, 'Юридическое лицо'),
))
method = models.PositiveSmallIntegerField(choices=(
(DEBET, 'Дебет'),
(CREDIT, 'Кредит'),
))
name = models.CharField(max_length=254)
owner = models.ForeignKey(User, related_name='w_accounts')
spectators = models.ManyToManyField(User, related_name='r_accounts')
when_opened = models.DateField()
@property
def balance(self):
assert self.method == self.DEBET
today = timezone.now()
rule = Rule.on_date(self.when_opened)
result = 0
current_date = self.when_opened
while current_date <= today:
result -= sum(x.total for x in self.cashing_set.on_date(current_date))
result -= sum(x.total for x in self.outcome_transfers.on_date(current_date))
result += sum(x.total for x in self.deposit_set.on_date(current_date))
result += sum(x.total for x in self.income_transfers.on_date(current_date))
result = 1 + rule.deposit_percent 0.01
current_date += timedelta(days=1)
return result
@property
def debt(self):
assert self.method == self.CREDIT
today = timezone.now()
rule = Rule.on_date(self.when_opened)
result = 0
Solution
In general this seems fine, easy to read, you seem to be following PEP8, that's excellent.
At first look, this repeated logic in
My immediate reaction is to extract those summations to a helper method, for example:
So that the above methods can reuse that with less duplication:
On closer look, these assert lines don't look good:
These assertions force the callers to check the value of
or to organize the code in a way that ensures that
It would be better to avoid such unwritten rules.
For example, it might be better to have one public property that applies the correct calculation based on the method:
Lastly, "debt" is misspelled in
At first look, this repeated logic in
balance and debt jumps in the eye:@property
def balance(self):
# ...
while current_date <= today:
result -= sum(x.total for x in self.cashing_set.on_date(current_date))
result -= sum(x.total for x in self.outcome_transfers.on_date(current_date))
result += sum(x.total for x in self.deposit_set.on_date(current_date))
result += sum(x.total for x in self.income_transfers.on_date(current_date))
# ...
@property
def debt(self):
# ...
while current_date <= today:
result += sum(x.total for x in self.cashing_set.on_date(current_date))
result += sum(x.total for x in self.outcome_transfers.on_date(current_date))
result -= sum(x.total for x in self.deposit_set.on_date(current_date))
result -= sum(x.total for x in self.income_transfers.on_date(current_date))
# ...My immediate reaction is to extract those summations to a helper method, for example:
def sum_total_transfers(self):
result = 0
result -= sum(x.total for x in self.cashing_set.on_date(current_date))
result -= sum(x.total for x in self.outcome_transfers.on_date(current_date))
result += sum(x.total for x in self.deposit_set.on_date(current_date))
result += sum(x.total for x in self.income_transfers.on_date(current_date))
return resultSo that the above methods can reuse that with less duplication:
@property
def balance(self):
# ...
while current_date <= today:
result += self.sum_total_transfers()
# ...
@property
def debt(self):
# ...
while current_date <= today:
result -= self.sum_total_transfers()
# ...On closer look, these assert lines don't look good:
@property
def balance(self):
assert self.method == self.DEBET
# ...
@property
def debt(self):
assert self.method == self.CREDIT
# ...These assertions force the callers to check the value of
self.method before using these properties,or to organize the code in a way that ensures that
.debt will only be called when self.method == self.CREDIT, otherwise the program would crash.It would be better to avoid such unwritten rules.
For example, it might be better to have one public property that applies the correct calculation based on the method:
@property
def balance(self):
if self.method == self.DEBET:
return self._credit_balance
return self._debt_balance
@property
def _credit_balance(self):
assert self.method == self.DEBET
# ...
@property
def _debt_balance(self):
assert self.method == self.CREDIT
# ...Lastly, "debt" is misspelled in
self.DEBET.Code Snippets
@property
def balance(self):
# ...
while current_date <= today:
result -= sum(x.total for x in self.cashing_set.on_date(current_date))
result -= sum(x.total for x in self.outcome_transfers.on_date(current_date))
result += sum(x.total for x in self.deposit_set.on_date(current_date))
result += sum(x.total for x in self.income_transfers.on_date(current_date))
# ...
@property
def debt(self):
# ...
while current_date <= today:
result += sum(x.total for x in self.cashing_set.on_date(current_date))
result += sum(x.total for x in self.outcome_transfers.on_date(current_date))
result -= sum(x.total for x in self.deposit_set.on_date(current_date))
result -= sum(x.total for x in self.income_transfers.on_date(current_date))
# ...def sum_total_transfers(self):
result = 0
result -= sum(x.total for x in self.cashing_set.on_date(current_date))
result -= sum(x.total for x in self.outcome_transfers.on_date(current_date))
result += sum(x.total for x in self.deposit_set.on_date(current_date))
result += sum(x.total for x in self.income_transfers.on_date(current_date))
return result@property
def balance(self):
# ...
while current_date <= today:
result += self.sum_total_transfers()
# ...
@property
def debt(self):
# ...
while current_date <= today:
result -= self.sum_total_transfers()
# ...@property
def balance(self):
assert self.method == self.DEBET
# ...
@property
def debt(self):
assert self.method == self.CREDIT
# ...@property
def balance(self):
if self.method == self.DEBET:
return self._credit_balance
return self._debt_balance
@property
def _credit_balance(self):
assert self.method == self.DEBET
# ...
@property
def _debt_balance(self):
assert self.method == self.CREDIT
# ...Context
StackExchange Code Review Q#116095, answer score: 7
Revisions (0)
No revisions yet.