views:

856

answers:

4

Here's a Django model class I wrote. This class gets a keyerror when I call get_object_or_404 from Django (I conceive that keyerror is raised due to no kwargs being passed to __init__ by the get function, arguments are all positional). Interestingly, it does not get an error when I call get_object_or_404 from console.

I wonder why, and if the below code is the correct way (ie, using init to populate the link field) to construct this class.

class Link(models.Model)

    event_type = models.IntegerField(choices=EVENT_TYPES)
    user = models.ForeignKey(User)
    created_on = models.DateTimeField(auto_now_add = True)
    link = models.CharField(max_length=30)
    isActive = models.BooleanField(default=True)

    def _generate_link(self):
        prelink = str(self.user.id)+str(self.event_type)+str(self.created_on)
        m = md5.new()
        m.update(prelink)
        return m.hexdigest()

    def __init__(self, *args, **kwargs):
        self.user = kwargs['user'].pop()
        self.event_type = kwargs['event_type'].pop()
        self.link = self._generate_link()
        super(Link,self).__init__(*args,**kwargs)
+4  A: 
self.user = kwargs['user'].pop()
self.event_type = kwargs['event_type'].pop()

You're trying to retrieve an entry from the dictionary, and then call its pop method. If you want to remove and return an object from a dictionary, call dict.pop():

self.user = kwargs.pop('user')

Of course, this will fail with a KeyError when "user" is not present in kwargs. You'll want to provide a default value to pop:

self.user = kwargs.pop('user', None)

This means "if "user" is in the dictionary, remove and return it. Otherwise, return None".

Regarding the other two lines:

self.link = self._generate_link()
super(Link,self).__init__(*args,**kwargs)

super().__init__() will set link to something, probably None. I would reverse the lines, to something like this:

super(Link,self).__init__(*args,**kwargs)
self.link = self._generate_link()

You might want to add a test before setting the link, to see if it already exists (if self.link is not None: ...). That way, links you pass into the constructor won't be overwritten.

John Millikin
+1  A: 

There's no reason to write your own __init__ for Django model classes. I think you'll be a lot happier without it.

Almost anything you think you want to do in __init__ can be better done in save.

S.Lott
and signaling, as shown in my answer, can emulate `__init__`
vikingosegundo
+1  A: 

wonder why, and if the below code is the correct way (ie, using __init__ to populate the link field) to construct this class.

I once got some problems when I tried to overload __init__ In the maillist i got this answer

It's best not to overload it with your own __init__. A better option is to hook into the post_init signal with a custom method and in that method do your process() and make_thumbnail() calls.

In your case the post_init-signal should do the trick and implementing __init__ shouldn't be necessary at all. You could write something like this:

class Link(models.Model)
    event_type = models.IntegerField(choices=EVENT_TYPES)
    user = models.ForeignKey(User)
    created_on = models.DateTimeField(auto_now_add = True)
    link = models.CharField(max_length=30)
    isActive = models.BooleanField(default=True)

    def create_link(self):
        prelink = str(self.user.id)+str(self.event_type)+str(self.created_on)
        m = md5.new()
        m.update(prelink)
        return m.hexdigest()

def post_link_init(sender, **kwargs):
    kwargs['instance'].create_link()
post_init.connect(post_link_init, sender=Link)

>>> link = Link(event_type=1, user=aUser, created_on=datetime.now(), link='foo', isActive=True)

providing keyword unique for link = models.CharField(max_length=30, unique=True) could be helpful, too. If it is not provided, get_object_or_404 may won't work in case the same value in the link-field exists several times.

signals and unique in the django-docs

vikingosegundo
+1  A: 

I don't think you need the __init__ here at all.

You are always calculating the value of link when the class is instantiated. This means you ignore whatever is stored in the database. Since this is the case, why bother with a model field at all? You would be better making link a property, with the getter using the code from _generate_link.

@property
def link(self): 
    ....
Daniel Roseman