Glad to be of assistance. Let me know when it's finished or if you want another look over,
Euan On 3 July, 15:22, Rainy <andrei....@gmail.com> wrote: > Thanks for great feedback, Euan. I've added some comments below. > > On Jul 3, 8:26 am, "euan.godd...@googlemail.com" > > <euan.godd...@gmail.com> 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 django-us...@googlegroups.com. To unsubscribe from this group, send email to django-users+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-users?hl=en.
