Thanks for great feedback, Euan. I've added some comments below. On Jul 3, 8:26 am, "[email protected]" <[email protected]> wrote: > I had a look over your tutorial and it looks pretty sweet. Definitely > gets down into explaining how to use the admin interface and taught me > things I hadn't realized. I have a few suggestions for you (feel free > to ignore :)): > > 1) I'm not sure how good an idea it is to call the attribute on your > DateTime model "datetime" - it conflicts with the datetime module. > Although this shouldn't cause any problems, it might be confusing.
Hm, maybe calling it 'created' is better.. but then you'll have created.created which also doesn't look that great. I generally avoid using names like list, tuple because they're built-ins and also because they get highlighted in Vim :). Haven't really thought about using module names.. In fact I don't think I ever ran into this before. I'll have to think about this! > > 2) In the "Changing Save Redirect" section you import a load of stuff > inside an instance method. I'd prefer to see this imported at module > level. I agree, that's what I do when I do actual development; but in this case I made an exception to keep the tutorial shorter. Generally my idea for this set of tutorials is to err on the side of maintaining momentum rather than doing things by the book, I'll expand on this idea more below. > 3) In the same section, you do request.POST.has_key. I believe the > preferred syntax is "name_of_key" in request.POST. That's straight from Django sources :). > 4) In the same section you are using hard-coded URLs - can you not get > these by using reverse() ? It might not be possible as I know the I want to introduce reverse() a bit later, in a larger tutorial where it will be more self-evidently useful. So, the goal is to have someone who's not a programmer go through the tutorial and feel that a useful and practical app was done with very little code and only explain enough concepts that are necessary for that particular app. Then, over the course of the whole set of tutorials, various concepts will be introduced at the exact point when they'd be most useful. On the other hand, I haven't actually used reverse() myself. I know that it can get a little tricky. I'll definitely try to see if it makes sense to introduce it in the view function. > admin is quite a beast. > 5) In the mark_done method on Item, you could definitely use reverse > and that should aid any changes to your URL structure. > 6) In the "Customizing DateTime" your unicode method returns a str not > unicode Good point - will fix that. > 7) In the same section you talk about an OnOff property. I think you > actually mean attribute (although in terms of the concept it is indeed > a property, just in terms of the model it is an attribute) Yes, that's where I have to balance use of language that sounds clearer vs. fine points of terminology. I feel that most programming tutorials are too quick to emphasize correct terminology, but I definitely get where that comes from, too.. > 8) In the "Adding users" section, I think you could replace the loop > with: > > Item.objects.filter(created=obj, > user_isnull=True).update(user=request.user). Good, this is actually new to me. I'll keep it in mind but I think for the first tutorial the loop is clearer and this is something that should be introduced when performance gain will be more apparent. > > This would use only one SQL UPDATE and not n SELECTs and n UPDATEs. > > 9) At the end when the possible actions are checked it might be nice > to raise an exception if you don't find a valid action. Here, in fact, I'm following Django tutorial where they at one point rely on the regex in urls.py and then explain that they're not validating because it would not get through the regex if it were wrong. On the other hand, depending on the project, I might raise an exception if I feel that an error in the regex might be hard to debug otherwise. > > I hope you find the above helpful. There's very little wrong with the > tutorial and it's really clear, but I think at least some of the > points might be well incorporated. Very helpful - even if I didn't follow all of the advice, just thinking and going through the reasons was extremely valuable. I really appreciated the detailed critique, and I'd be very thankful if you also looked at my future tutorials I'll also post here. -ak -- You received this message because you are subscribed to the Google Groups "Django users" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/django-users?hl=en.
