search

zackmdavis / Finetooth

an experimental discussion forum
MIT License
2 stars 0 forks source link

Reynolds v. Sims #42

Closed zackmdavis closed 9 years ago

zackmdavis commented 9 years ago

@cookjw review plz

zackmdavis commented 9 years ago

Like, do you think the "setting the current user as an attribute on each post so that render can still be a model (mixin) method" hack is OK, or should we search for a better way to do things? If it is okay, is there some way to make it less verbose and easy-to-forget-on-a-new-view—I guess that could go in our scored_context helper, but that mechanism is itself somewhat questionable (and would probably mean the end of our brief consideration of class-based views, for better or for worse).

cookjw commented 9 years ago

Why is the scored_context mechanism questionable? Also, I don't (yet?) understand the point of class-based views.

zackmdavis commented 9 years ago

Well, I must have been confused or forgetful when I wrote the previous comment; scored_context is OK. (I mean, I don't remember seeing such a "pass the entire prospective context dictionary to a function before actually giving it to render" pattern before, but that doesn't make it illegitimate.)

What is very questionable, on the other hand (and what I may have been trying to think of in the previous comment), is the @paginated_view/paginated_context hack—it's awfully DAMP ("duplicated areas of my program") to have to decorate a view and call a function (paginated_context) on the context-to-be in order for some piece of functionality to work! Whereas even though I don't really like class-based views (brittle and opaque magic is the worst kind; I mostly used it for the monthly archives view to cement my status as a true Djangonaut), I was surprised at how much code it took for me to implement pagination myself even with the Paginator, and ListView gives you that functionality nearly for free ...

Anyway, I have some ideas about how to improve @paginated_context and scored_context, so I'll probably end up doing that and convert the archives view away from ListView.

cookjw commented 9 years ago

OK, since the overlapping-vote-prevention mechanism appears to work, I'm just going to go ahead and merge this on the assumption that the improvements to which you allude can be made in a future version over which we'll have some degree of control...

zackmdavis commented 9 years ago

:shipit: