Newbie question: How to avoid a very long view function?

[Cheng Guo]

Hello,

I am new to Django and I have run into an issue with views.py.  I understand that there is a function behind each view. For a view that I am currently writing, it accepts a file upload from user and stores the file on the server. To do that, I need to:

1. check the file extension is correct

2. generate a SHA-1 id for the file based on its content

3. write the file to disk

4. save information about the file to database

(oh, I also created two global variables as well)

As you can see, if more features are added, the list goes on. It makes this function very long. As the number of views grow, the views.py file will become bloated. I wonder what would be the ideal way to deal with this?

Something that I can think of but not sure if it is correct:

- divide the long function up into smaller functions

- store these smaller functions in a different .py file

Let me know your approach, thanks!

[Mike Dewhirst]

Absolutely correct. But first create a views directory (complete with
__init__.py file therein) to completely replace your views.py file.

Then any number of files can contain your views ...

from app.views.this import That

... where this.py has a class That

>
> Let me know your approach, thanks!
>

> --
> You received this message because you are subscribed to the Google
> Groups "Django users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to django-users...@googlegroups.com
> <mailto:django-users...@googlegroups.com>.
> To post to this group, send email to django...@googlegroups.com
> <mailto:django...@googlegroups.com>.
> Visit this group at http://groups.google.com/group/django-users.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-users/bf9f6ff3-a35a-4f00-8537-c6fd66d07f8d%40googlegroups.com
> <https://groups.google.com/d/msgid/django-users/bf9f6ff3-a35a-4f00-8537-c6fd66d07f8d%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

[Daniel França]

If the operations are model related, why don't move some of those functions to models.py.

> an email to django-users+unsubscribe@googlegroups.com
> <mailto:django-users+unsub...@googlegroups.com>.

> To post to this group, send email to django...@googlegroups.com

> <mailto:django-users@googlegroups.com>.

> Visit this group at http://groups.google.com/group/django-users.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-users/bf9f6ff3-a35a-4f00-8537-c6fd66d07f8d%40googlegroups.com
> <https://groups.google.com/d/msgid/django-users/bf9f6ff3-a35a-4f00-8537-c6fd66d07f8d%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

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

To unsubscribe from this group and stop receiving emails from it, send an email to django-users+unsubscribe@googlegroups.com.
To post to this group, send email to django...@googlegroups.com.

Visit this group at http://groups.google.com/group/django-users.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/54BCB642.8040201%40dewhirst.com.au.

[Cheng Guo]

Great, thanks!

> <mailto:django-users+unsub...@googlegroups.com>.

[James Schneider]

I second moving some (or most) of the functionality to models.py. For instance, calculating the SHA value should be done as part of the model's save() function, not done in the view.

Convention dictates that the only code that is placed in the view itself should be logic related to prepping the templates for rendering/displaying things, such as setting variables in the template context.

Any operations related to 'business logic' such as data manipulation (ie save behavior) as it relates to specific objects should be handled by the object/model itself. Ideally, the model instance should be smart enough to save itself (or perform some other action) on its own without any input from the view, assuming the necessary values for the operation are provided from some other source.

Obviously this is only a convention and not a rule, but it will likely serve you better in the long run. In general, views tend to be quite short. My class-based views just have a couple of class-level attributes set, if anything at all (although they rely heavily on inherited behavior from parent class views, which are much more complicated).

-James

To unsubscribe from this group and stop receiving emails from it, send an email to django-users...@googlegroups.com.

To post to this group, send email to django...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-users.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/CACPst9JZnMGK8vS00G1oqwEVt9qbf3TnTsdoJCwdy-mN02qEPw%40mail.gmail.com.

[Cheng Guo]

So in my case, I need to generate a unique id for the file and save it to disk.

I have a model called UploadFile, so you recommend to add two class methods to the UploadFile model, like the following?

class UploadFile(models.Model):

    @classmethod

    def generate_id():

        pass

    @classmethod

    def save_to_disk():

        pass

> an email to django-users...@googlegroups.com
> <mailto:django-users+unsubscrib...@googlegroups.com>.

> To post to this group, send email to django...@googlegroups.com

> <mailto:django...@googlegroups.com>.

> Visit this group at http://groups.google.com/group/django-users.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-users/bf9f6ff3-a35a-4f00-8537-c6fd66d07f8d%40googlegroups.com
> <https://groups.google.com/d/msgid/django-users/bf9f6ff3-a35a-4f00-8537-c6fd66d07f8d%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

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

To unsubscribe from this group and stop receiving emails from it, send an email to django-users...@googlegroups.com.

[James Schneider]

I wouldn't decorate them as class methods. You would want to call them from the objects themselves. For the save_to_disk() method, I was actually referring to the Django save() method (https://docs.djangoproject.com/en/1.7/topics/db/models/#overriding-predefined-model-methods).

As you have it now, it wouldn't work since you would need to call UploadFile.generate_id(), but you don't have an available argument to pass in the UploadFile object to get the ID for. 

If you remove the decorator, you would call it on the object itself:

class UploadFile(models.Model):

    def generate_id(self):

        # pseudocode to do stuff to generate ID using self

        # import hashlib

        # self.file_hash = hashlib.sha1(self.file, 'rb') 

        # return the ID or None if it fails

        # return self.file_hash or None

    # override the model save method to generate the hash ID, then save for real
    def save(self):
        self.generate_id()
        super(UploadFile, self).save(*args, **kwargs)

You probably don't need to change your view logic at all in this case (unless you have code that generates the hash and attaches it to the object, then remove it and let the object handle that itself). Basically, when your UploadFile object is saved, it will call the save() method on the object, which in turn will generate the ID based on the file, attach it to the object, and then save the object to the database.

Note that this will run the hashing mechanism every time the object is saved, whether the file it points at changes or not. If this isn't what you want, I would add an extra checks in generate_id() to account for all of the scenarios where you wan the hash to update (every time the object is saved vs. only when the file changes vs. only generated once even with a file change, etc.). 

In a typical scenario where the hash is run every time the object is saved, the object itself would create the hash. This way you can save the object from any view (or from the shell), and it would always generate a new hash automagically (new meaning recalculated, may end up the same as the previous value). 

TL;DR; The hash shouldn't be calculated external to the file that is attached to the object, it should be calculated by the object itself.

Hope that helps and makes sense...

-James

To view this discussion on the web visit https://groups.google.com/d/msgid/django-users/80aef78c-9cec-4e50-be11-bc506d019b8c%40googlegroups.com.

[Daniel Roseman]

On Monday, 19 January 2015 07:28:14 UTC, Cheng Guo wrote:

Hello,

I am new to Django and I have run into an issue with views.py.  I understand that there is a function behind each view. For a view that I am currently writing, it accepts a file upload from user and stores the file on the server. To do that, I need to:

1. check the file extension is correct

2. generate a SHA-1 id for the file based on its content

3. write the file to disk

4. save information about the file to database

(oh, I also created two global variables as well)

Others have pointed out ways to split this up, but I wanted to pick up on the last sentence. You really really should not be setting global variables in your view, especially in response to user input. Global variables are shared by all requests in a process, and you can't guarantee either that all requests will be from the same user, or even that the same user will hit the same process in subsequent requests. Don't do this: if you need to store state between requests, use the db or the session.

--

DR.

[Cheng Guo]

Hello James:

Thank you very much for spending the time reading and answering this question， really appreciated!

I complete understand your suggestion of overriding the "save" method in the "UploadFile" to generate the id before saving. Thank you for pointing this out.

On the other hand, I read that it is not recommended to store files in database. So I decide to only store upload files on disk and save their file path and generated id in the database. So here is my model:

class UploadFile(models.Model):
    upload_date = models.DateTimeField('timestamp', auto_now_add=True)
    original_file_name = models.CharField(max_length=200)
    file_id = models.CharField(max_length=40, primary_key=True)
    file_path = models.CharField(max_length=500)
    owner = models.ForeignKey(User)

I could use your method to override the save method, but the problem I face is that to generate the id, I need to pass the "UploadedFile" object to the  model, like this:

class UploadFile(models.Model):

    def generate_id(self, uploaded_file):

         import hashlib

         hasher = hashlib.sha1()

         for chunk in uploaded_file.chunks():

             hasher.update(chunk)

         

         self.file_id = hasher.hexdigest()

    def save(self, upload_file):    <------- this is not gonna work since 'save' only takes (self) as a parameter
        self.generate_id(upload_file)
        super(UploadFile, self).save(*args, **kwargs)

So I have two ways to solve this:

1. generate the id in the view, and simply pass it to the UploadFile's constructor, then call save

2. create a variable in UploadFile:

    class UploadFile(models.Model):

        ...

        self.file = None

         def generate_id(self, uploaded_file):

             import hashlib

             hasher = hashlib.sha1()

             for chunk in uploaded_file.chunks():

                 hasher.update(chunk)

         

             self.file_id = hasher.hexdigest()

        def save(self):    
            self.generate_id(self.file)
            super(UploadFile, self).save(*args, **kwargs)

in my view:

  

     def upload_view(request):

         file = request.FILES['file']

         upload_file = UploadFile(...)

         upload_file.file = file

         upload_file.save()

Do you think this is a good solution?

[Cheng Guo]

Thank you Daniel, I didn't know this! I am going to lookup how to use session. Thanks!