Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#23491 closed Cleanup/optimization (fixed)

Django Tutorial 1.7, Pt 3. Adding Index method to views.py

Reported by: derrick kearney Owned by: nobody
Component: Documentation Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

First off I appreciate the work and effort done. Thank you.
I am confused in part 3 of the tutorial when the index method is added. Looking at the code it appears that everything previous should be replaced ( the methods detail, results and vote). But running the code without the detail method generates an error.

In part 2 of the tutorial when code was added you used:

# ... 

seemingly to indicate to add to the existing code. No such notation is done for:

from django.http import HttpResponse

from polls.models import Question


def index(request):
    latest_question_list = Question.objects.order_by('-pub_date')[:5]
    output = ', '.join([p.question_text for p in latest_question_list])
    return HttpResponse(output)

And looking at it on its own, it looks complete. If the views.py is supposed to have a details method in the code why is it not there? Django is great with documentation and this is why I find it so perplexing that the entire file contents up to that point cannot be shown.

Thank you.

Derrick

Attachments (1)

23491.diff (1.1 KB) - added by Tim Graham 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by Baptiste Mispelon

Easy pickings: set
Triage Stage: UnreviewedAccepted

Hi,

Thanks for your feedback. I agree with you that we could improve this bit to make it less confusing.

How about simply adding a # ... comment at the end, like so:

from django.http import HttpResponse

from polls.models import Question


def index(request):
    latest_question_list = Question.objects.order_by('-pub_date')[:5]
    output = ', '.join([p.question_text for p in latest_question_list])
    return HttpResponse(output)

# ...

Or is that not obvious enough?

Thanks

comment:2 Changed 8 years ago by Aymeric Augustin

The tutorial must be absolutely explicit. I don't think # ... suffices.

comment:3 Changed 8 years ago by derrick kearney

That would help, but I think including the whole file would really help. In my opinion tut 3 is definitely a step up in complexity from tut 2, and clarity is key. In the end it is only adding a few more lines of html to the source, and that would make it perfectly clear.

Thank you for your response, and again great job on helping out the overall Django project aaugustin. :)

derrick

comment:4 Changed 8 years ago by Tim Graham

I don't think including all the previous code is a good idea. It will make it more difficult to see the new code that's being added and encourage users to simply copy and paste. The current version has worked for 8 years or so, so I don't think we need radical changes.

comment:5 Changed 8 years ago by derrick kearney

Tim,
I respectfully disagree. Just because something has been in place for a period of time does not make it correct. You are looking at this from a perspective of someone who knows exactly what to do, this tutorial is for someone who has never used Django before. We are not talking about a file 80 or more lines where it would be difficult to see the addition of the index method. And the user is going to easily see that thier existing file views.py differs. And again that is part of problem, it does not say to add it, there is no direction about what to do period. In some cases up to that point in the tutorial all the contents of a file are replaced, so in this case because there was no explanation it just looks like that is it a complete file. A new user should not have to guess, it should be explicit in the spirit of Python. And I have to say, what I am asking for, and aaugustin suggested as well, is far from radical. I am not asking for a complete change of the page, just clarification to help. As written it is not clear.

derrick

Changed 8 years ago by Tim Graham

Attachment: 23491.diff added

comment:6 Changed 8 years ago by Tim Graham

Proposed patch is attached above. I'm not against clarification, just the idea that we should include the entire file every time a change is made.

comment:7 Changed 8 years ago by Aymeric Augustin

Triage Stage: AcceptedReady for checkin

Yes, that's a good compromise.

comment:8 Changed 8 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 54fd84e43230bc739160b680c5ba51542e2e6e7d:

Fixed #23491 -- Clarified tutorial 3.

Thanks diek for the suggestion.

comment:9 Changed 8 years ago by Tim Graham <timograham@…>

In fdcef1b86359b1c658b58fdfc6bfdebc11f830f6:

[1.7.x] Fixed #23491 -- Clarified tutorial 3.

Thanks diek for the suggestion.

Backport of 54fd84e432 from master

comment:10 Changed 8 years ago by Tim Graham <timograham@…>

In aa1939c1d1e923d98fb00250dee34e00421bcee5:

[1.6.x] Fixed #23491 -- Clarified tutorial 3.

Thanks diek for the suggestion.

Backport of 54fd84e432 from master

comment:11 Changed 8 years ago by derrick kearney

You are welcome. It was my pleasure to help. Take care.

Note: See TracTickets for help on using tickets.
Back to Top