search

RyanBalfanz / django-sendgrid

SendGrid for Django
http://pypi.python.org/pypi/django-sendgrid
97 stars 26 forks source link

Improve SendGridMessage inheritance and code style #1

Closed blanchardjeremy closed 12 years ago

blanchardjeremy commented 12 years ago

Continuing the discussion we started here: 93f1ce1

Specifically in regard to the most recent commit: fb910ce

blanchardjeremy commented 12 years ago

Here are my thoughts:

I'd change the way you do the inheritance to have the super() calls in the mixin. See this example: http://dpaste.org/b7rQx/

Also change the base class to inherit from object. I think that's a best practices thing.

I might get around to whipping up a pull request.

RyanBalfanz commented 12 years ago

I didn't think mixin should inherit from anything? Consider http://hg.python.org/cpython/file/2.7/Lib/UserDict.py.

I'm not sure if super() can be moved into the mixin because of it's first arg (http://docs.python.org/library/functions.html#super), but the connection stuff certainly can be, as well as the log message and signal.

RyanBalfanz commented 12 years ago

Latest: f8874054ed04fca8cd5c1bf1e37bfb525e64eed8

blanchardjeremy commented 12 years ago

At this point I'm just satisfying my own curiosity about best practices and how inheritance should/can work. :)

Yeah that's a cleaner solution. (also if you reference this issue in your commit with "#1", it will automatically show up here)

Every mixin in Django core inherits from (object), that's why I thought of it as a best practice. See https://github.com/django/django/blob/master/django/views/generic/edit.py#L9

I could've sworn there was a way to call super() from the mixin, but apparently not. Even though my example app works, I can't get it to work with your code.

I think what we have now is more than good enough. It's been fun to poke at. :)