keep models simple ? where to put the business logic ?

[Michael Palumbo]

Hi,

I know this has already been discussed several times, I found several posts about it through Google but I'm still interested in getting your opinion on an example.

I'm wondering that because my models file is getting big. That makes me confused so I'm wondering if I'm doing the right thing from a design point of view.

I have the feeling that my models should remain simple. What do you think ?

For example, let's say I want to create a model named Feed. (simplified version)

class Feed(models.Model):

    name = models.CharField(max_length=255, unique=True)

    url = models.URLField(unique=True)

    etag = models.CharField(max_length=255, null=True, blank=True)

I want to be able to extract a feed (that is to say to download it and store it(as a file but I also keep a track in the DB through a File model)). Would you create:

- an extract method in the model (this is what I have for now, I simplified it, it is much bigger actually. I have some try..except block to log some possible errors, etc. And my _store function is pretty big too because it saves in the storage and in the DB.

   def extract(self):

        data, etag = utils.download_from_url(self.url, self.etag) #Download from the url

        self._store(data) #Store it

        self.etag = etag # Change the etag

        self.save()

to use like that:

f = Feed.objects.get(pk=1)

f.extract()

- a view:

that I could access through http://127.0.0.1:8000/feed/extract/1

- a "util" function to whom I pass the Feed object.

f = Feed.objects.get(pk=1)

utils.extract_feed(f)

Thanks for your advice.

Michael

[bruno desthuilliers]

On Apr 22, 7:51 pm, Michael Palumbo <michael.palumb...@gmail.com>
wrote:

> Hi,
>
> I know this has already been discussed several times, I found several posts
> about it through Google but I'm still interested in getting your opinion on
> an example.
>
> I'm wondering that because my models file is getting big. That makes me
> confused so I'm wondering if I'm doing the right thing from a design point
> of view.
> I have the feeling that my models should remain simple. What do you think ?

https://www.google.com/search?q=anemic+domain+model

> For example, let's say I want to create a model named Feed. (simplified
> version)
> class Feed(models.Model):
>     name = models.CharField(max_length=255, unique=True)
>     url = models.URLField(unique=True)
>     etag = models.CharField(max_length=255, null=True, blank=True)
>
> I want to be able to extract a feed (that is to say to download it and
> store it(as a file but I also keep a track in the DB through a File
> model)). Would you create:
> - an extract method in the model

That's probably what I would do.

>
> - a view:

Nope. The view should just deal with user interactions (in this case,
allow a user to launch the extraction).

FWIW, a part of the Django code I see suffer from this problem (anemic
domain, and anemic forms to), and it's a major PITA when you want to
extend such a code, because you have business logic and user
interaction logic deeply mixed in the views for no good reason.

> - a "util" function to whom I pass the Feed object.
> f = Feed.objects.get(pk=1)
> utils.extract_feed(f)

How is this better than having the very same function being a method
of the model ?

Models - like any other module - should indeed be "as simple as
possible", _but not simpler_. If you concern is about your models.py
file size, it might be time to refactor it into a package.

Now if there are parts of your methods that are low-level enough and
don't really need to know about your models, yeps, they may belong to
some "model-agnostic" utility module. Refactoring methods this way can
help keeping the method code hi-level and readable.

My 2 cents

[Javier Guerra Giraldez]

On Mon, Apr 23, 2012 at 6:47 AM, bruno desthuilliers
<bruno.des...@gmail.com> wrote:
> Models - like any other module - should indeed be "as simple as
> possible", _but not simpler_. If you concern is about your models.py
> file size, it might be time to refactor it into a package.

or maybe the app has to be more focused, and split in two or more apps.

> Now if there are parts of your methods that are low-level enough and
> don't really need to know about your models, yeps, they may belong to
> some "model-agnostic" utility module.

also when the usercase concepts are not exactly the same as the
database records. then another model-like layer can be useful.

for example, lets say you're working with a genealogy application, and
you have a Person model, and several kinds of relationships between
person instances. But let's also say that you have a 'Family'
concept that is easy to derive from the database (so it doesn't need
another model class), but you want to add some extra behaviour at the
Family level (above Person and Relationship). then it might be
useful to add a new Family.py module that works on your models and is
managed by the views in similar ways to them.

but in the end, yes: the vast majority of business logic belongs in
models.py files, definitely not in the views.

--
Javier

[Daniel Sokolowski]

To alleviate models.py file growing large you can split it up like so

/models/
/models/__init__.py
/models/invoices.py
/models/products.py
/models/taxes.py
...

The are two tricks to the above to make it work however; first you must
import your files in the __init__.py and manually specify the app_label for
all your model files:

# /models__init__.py
from .taxes import *
from .products import *
from .sandh_costs import *
from .discounts import

# /models/taxes.py
class DiscountBase(models.Model):
...
### model options - "anything that's not a field"
class Meta:
app_label = 'pnpstore' # must specify same app label so that in
admin models are grouped correctly

I also split views in a similar manner.

--
Javier

--
You received this message because you are subscribed to the Google Groups
"Django users" group.
To post to this group, send email to django...@googlegroups.com.
To unsubscribe from this group, send email to
django-users...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-users?hl=en.

Daniel Sokolowski
Web Engineer
KL Insight
http://klinsight.com/
Tel: 613-344-2116 | Fax: 613.634.7029
993 Princess Street, Suite 212
Kingston, ON K7L 1H3, Canada

[Michael Palumbo]

Hi guys,

Thanks for your answers, it helps.

- Anemic domain model : I didn’t know about this before, it is good to know.
- Daniel : if I split my models and import all of them in my __ini__.py file, why do I still need to use the app_label meta ?
I know I have to because when I run the syncdb command, it does not work unless I specify the app_label meta but I’m really wondering why since I can still access to them normally like it would be with one file: “from my_app.models import model1”.

So now I’m convinced that business logic should remain in the models, my doubts are gone now.
However, my feeling of having a complex models might also come from the fact that I put some numerous try..except blocks inside my function and log the errors.
So my question is: when raising and catching errors and when logging ?
We could say that catching errors and logging are not part of the business logic either and they should thus be on a higher level ?

Should I catch the errors inside my extract function and log them.
Or should I wrap the call of feed.extract() with a try..except block and log them ?
Maybe it depends on the nature of the errors ?

For example, the download_from_url method will success only if there is data to return. For example, if the etag is different, it raises a DataNotModified exception, etc.

For now, I catch it, log it, and return None on my extract method.
def extract(self):
       try

              data, etag = utils.download_from_url(self.url, self.etag) #Download from the url

              f = self._store(data) #Store it
       except (DataNotModified as e):
              logger.warning(e)
              return none
       except (X as e):
              logger.error(e)
              return none
       except (Y as e):
              logger.info(e)
              return none
        

       self.etag = etag # Change the etag 

       self.save() 

       return f

What do you think ?

Thanks
Best,

[Michael Palumbo]

Any suggestions about catching and logging errors right in the models functions ?

[bruno desthuilliers]

On Apr 24, 1:32 pm, Michael Palumbo <michael.palumb...@gmail.com>
wrote:

> Hi guys,
>
> Thanks for your answers, it helps.
>
> - Anemic domain model : I didn’t know about this before, it is good to know.
> - Daniel : if I split my models and import all of them in my __ini__.py
> file, why do I still need to use the app_label meta ?

1/ tables prefix
2/ all management commands working on apps

> However, my feeling of having a complex models might also come from the
> fact that I put some numerous try..except blocks inside my function and log
> the errors.
> So my question is: when raising and catching errors and when logging ?
> We could say that catching errors and logging are not part of the business
> logic either

Why so ?

> and they should thus be on a higher level ?

Depends... As a general rule, only catch exceptions you can handle,
else let them propagate. There are a few cases where you want to catch
some exception you cannot directly handle:

1/ when you want to log some contextual info before re-resaising the
original exception

2/ when you want replace the original exception by your own to "hide"
some low-level implementation detail and/or help the calling code
handling this particular error. In both cases, don't assume to much
about what really happened in the first place and keep as much
debugging info as possible.

Also, remember that django already comes with a top-level exception
handler around your views, so the worst thing that may happen is an
HTTP 500.

[bruno desthuilliers]

On Apr 23, 3:45 pm, Javier Guerra Giraldez <jav...@guerrag.com> wrote:
> On Mon, Apr 23, 2012 at 6:47 AM, bruno desthuilliers
>

> <bruno.desthuilli...@gmail.com> wrote:
> > Models - like any other module - should indeed be "as simple as
> > possible", _but not simpler_. If you concern is about your models.py
> > file size, it might be time to refactor it into a package.
>
> or maybe the app has to be more focused, and split in two or more apps.

Possibly, but sometimes the complexity is inherent to the domain and
trying to split the app just makes things more complicated for no good
reason.

> > Now if there are parts of your methods that are low-level enough and
> > don't really need to know about your models, yeps, they may belong to
> > some "model-agnostic" utility module.
>
> also when the usercase concepts are not exactly the same as the
> database records. then another model-like layer can be useful.
>
> for example, lets say you're working with a genealogy application, and
> you have a Person model, and several kinds of relationships between
> person instances.   But let's also say that you have a 'Family'
> concept that is easy to derive from the database (so it doesn't need
> another model class), but you want to add some extra behaviour at the
> Family level (above Person and Relationship).   then it might be
> useful to add a new Family.py

please make it lowercase ;)

> module that works on your models

"model" is kind of an overloaded term here - there's the Django
"Model" class, and there's the "domain model" layer. As far as I'm
concerned, it all belongs to the "models" module / package, even if
there's not a single "Model" class involved.

> but in the end, yes: the vast majority of business logic belongs in
> models.py files, definitely not in the views.

Indeed.

> --
> Javier

[Daniel Sokolowski]

Michael, I’m split on this – I think it really depends on the situation. If these access methods are like low level api to be used in higher end code I would then throw a custom error and use debug level log only, then in your view I would catch an error when NoDataReturned and log it as warning.

 

Perhaps the question to ask is: what data set I want this method to return.  Either homogenous: always a ‘data’ of certain type (with a possibility of an Exception), or heterogeneous that is either the ‘data’ OR None, if it’s the latter you are using if/elif blocks if it’s the former you are using try/except.  A homogenous data set is cleaner and easier to debug in my opinion for such a highly dynamic language as Python. (for opposite see Haskell).

 

From: Michael Palumbo

Sent: Friday, April 27, 2012 6:17 AM

To: django...@googlegroups.com

Subject: Re: keep models simple ? where to put the business logic ?

--

You received this message because you are subscribed to the Google Groups "Django users" group.

To view this discussion on the web visit https://groups.google.com/d/msg/django-users/-/xIv_DaMs02gJ.

To post to this group, send email to django...@googlegroups.com.
To unsubscribe from this group, send email to django-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-users?hl=en.

 

Daniel Sokolowski
Web Engineer
Danols Web Engineering
http://webdesign.danols.com/

[HarpB]

First of all, I think your function is too big if it is doing all of these things. Are you able to unit test the function?

In my view, the logic for extract should not be in the model. I would create a class specifically for data extractions. I am assuming that the _store does not DB saves to the Feed model, so that logic should not be inside Feed. A model should only perform operations on its own model. If you want to do operations on 2 different models, then have a helper function to do so or put it in the manager of the model, not the model itself.

In your case, since the data is stored in 2 different storages, I would put the logic in a helper class inside helpers.py.
# helpers.py
class Feeder(object):

def store(data):

#  logix for storing to DB and any other storage.

@classmethod

def extract(cls, url, etag):

try:

#sub-logic here
return data, etag

except (.., ..) as ex:

# handle exceptions

@staticmethod

def extract_feed(feed)

data, etag = Feeder.extract(feed.url, feel.etag)
feed.update_etag(etag)

# models.py
class Feed():

def update_etag(self, etag):

if self.etag != etag:

self.etag = etag
self.save()

I strive for modular code and put business logic in its own classes. I unit test each code, so smaller the function, easier to test it and it is much more easier to use the code. In the above code, you could test it Feeder class without having a Feed model. As a side benefit, by having logic be independent of a specific model, you are able to much more easily use same piece of code for multiple purposes. i.e If you wanted to allow extracting for a specific url and etag from a VIEW or a specific feed from a view, you would be able to do both.

Overall, it comes down to what makes sense to you. Once you accept a certain way of coding, it is meant to PREVENT YOU from doing guess work as to where you placed a specific logic. So when a bug happens and need to find the code, you are not opening/closing files to find the logic, you know where you put the logic.