Bank account model implementation in Django

Question

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
 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.credit_percent
 current_date += timedelta(days=1)
 return result
class OperationManager(models.Manager):
 def on_date(self, date):
 return (super(OperationManager, self).get_queryset()).filter(when__gte=date, when__lte=date)
class MoneyTransfer(models.Model):
 sender = models.ForeignKey(BankAccount, related_name='outcome_transfers')
 receiver = models.ForeignKey(BankAccount, related_name='income_transfers')
 when = models.DateTimeField()
 total = models.FloatField()
 comment = models.CharField(max_length=255)
 objects = OperationManager()
class Deposit(models.Model):
 account = models.ForeignKey(BankAccount)
 total = models.FloatField()
 banker = models.ForeignKey(User)
 when = models.DateTimeField()
 objects = OperationManager()
class Cashing(models.Model):
 account = models.ForeignKey(BankAccount)
 total = models.FloatField()
 banker = models.ForeignKey(User)
 when = models.DateTimeField()
 objects = OperationManager()

Answer 1

In general this seems fine, easy to read, you seem to be following PEP8, that's excellent.

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 result

So 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.

Answer 2

You may want to look into custom querysets in Django for your Rule.on_date static method.

@staticmethod
def on_date(_date):
 return Rule.objects.get(start_date__lte=_date, end_date__gte=_date)

This appears to get all rules that are in effect on a certain date (docstrings might be useful, that's a guess), which is likely a common query that needs to be done. Usually helper methods for queries, such as this one, would go on the manager instance, allowing it to be called like

 Rule.objects.on_date(some_date)

Instead of putting it as a static method on the model. You can see examples of this in the User manager. This can be accomplished by either making a custom Manager or a custom QuerySet, and associating it with your model. I personally typically make the QuerySet work as the Manager, so I can use it anywhere within a query.

from django.db import models
class RuleQuerySet(models.QuerySet):
 def on_date(_date):
 return self.get(start_date__lte=_date, end_date__gte=_date)
class Rule(models.Model):
 start_date = models.DateField()
 end_date = models.DateField()
 deposit_percent = models.FloatField()
 credit_percent = models.FloatField()
 objects = RuleQuerySet.as_manager()

You are actually doing something similar to this further down, in your OperationManager, but you are extending from a Manager there instead of a QuerySet. If it's possible for there to be multiple rules for a day, you might also want to consider switching self.get to self.filter.

• Vassily: thank you! It's so helpful! Unfortunately, I have no enough reputation for voting up...