4

In my Django code, for OrderedArticle objects I need to compute a hist_price which is the multiplication of 2 fields: hist_unit_price * quantity.

The first way I did it was a simple property:

class OrderedArticle(Model):
    @property
    def hist_price(self):
        return self.hist_unit_price * self.quantity

Then, I realised that when I need to make extensive computing of these prices, I can't use this property for performance reason, and instead I must compute hist_price at a database level. That's why I wrote a custom queryset for this:

class OrderOperationQuerySet(Queryset):

    @staticmethod
    def _hist_price(orderable_field):  # can be an OrderedArticle or another object here
        return ExpressionWrapper(
            F(f'{orderable_field}__hist_unit_price') * F(f'{orderable_field}__quantity'),
            output_field=DecimalField())

Currently, both hist_price property and _hist_price queryset are used in my code.

Question

This works well, but I'm annoyed to write the same business logic twice. I have a feeling I'm not doing it "the right way" here. I think I should ensure at a code level, that no matter if I use the property or the queryset, it always returns the same result. In this specific case, the business logic is a simple multiplication between two decimals, so it should be OK, but I'll have other cases in my code where it's way more complex.

Do you see a way to improve my code? Thanks.

David Dahan
  • 10,576
  • 11
  • 64
  • 137

1 Answers1

1

This idea is similar to "hybrid attributes" from SQLAlchemy which have been asked about before - I wasn't super satisfied with any of the answers from that chain of threads (stuff like storing this computed value in an extra field on the table and always making sure to keep it updated).

You could have some internal function that your property and ExpressionWrapper function both use, as long as the required operators are overloaded to accept either the actual values or the F() objects (e.g. basic mathematical operators).

def multiplication(x, y):
    return x * y  # trivial here but it could be any mathematical expression really


def _hist_price(orderable_field):
    return ExpressionWrapper(
        multiplication(
            F(f"{orderable_field}__hist_unit_price"),
            F(f"{orderable_field}__quantity")
        ),
        output_field=DecimalField()
    )

@property
def hist_price(self):
    return multiplication(self.hist_unit_price, self.quantity)

If it gets more complex than basic numerical operations in one of these hybrid functions and you want to avoid duplicate business logic, you would need to write a wrapper func that could resolve to the correct output using the python function for the property caller, and the function that can operate on F objects for the query-set caller to maintain operator overloading. But this will lead to code which introspects arguments to work out what to do, which can be unintuitive, so it's a trade off either way really.

In rough pseudocode one of these custom functions would be like

def _hybrid_lower(value):
   if isinstance(value, F):  # maybe F would be sufficient or some other class higher in the hierarchy
       # https://docs.djangoproject.com/en/2.2/ref/models/expressions/#func-expressions
       return Func(value, function='LOWER')
   else:
       return value.lower()

and then you could use this custom function in your function that the property and queryset both call. Some duplication of code might not be the worst trade-off if you do start needing really complex functions as both database operations and Python.

Rach Sharp
  • 2,324
  • 14
  • 31
  • Thanks, very interesting answer. I'm not sure about using the first solution because it only abstracts the multiplication, not the whole business logic. I mean, even with the `multiplication` function, I still would be able to multiply the wrong fields together in both `hist_price` and `_hist_price`. – David Dahan Apr 15 '19 at 12:55