patternpythondjangoMinor
Optimising and condensing Django View Class
Viewed 0 times
djangoviewoptimisingcondensingandclass
Problem
This follows on from a previous question I asked which was of huge help to me. I am still only learning Python and Django and I would love to learn how to code better.
The below is a
The below is designed to show the user a different image, selected at random from
In the previous question I asked shortening the code proved to be a simple matter as each of my
Has anyone got any suggestions for optimising, and preferably condensing the below code? Remember I am still learning so any suggestion would be helpful.
views.py
```
path_one_images = ['P1D1.jpg', 'P2D2.jpg', 'P3D3.jpg', 'P4D4.jpg', 'P5D5.jpg', 'P6D6.jpg', 'P7D7.jpg', 'P8D8.jpg', 'P9D9.jpg']
class SurveyWizardOne(SessionWizardView):
def get_context_data(self, form, **kwargs):
context = super(SurveyWizardOne, self).get_context_data(form, **kwargs)
if self.steps.current in ['5','6','7','8','9','10','11','12','13','14','15','16']:
print '\nThe available list of Path_One images is', path_one_images
global first_image, second_image, third_image, fourth_image, fifth_image, sixth_image, seventh_image, eight_image, ninth_image
if self.steps.current == '5':
first_image = random.choice(path_one_images)
path_one_images.remove(first_image)
display_image = first_image
The below is a
get_context_data method which is used to return a variable to an internal page of a Django SessionWizardView. The SessionWizardView splits a lengthy form over multiple pages. The below is designed to show the user a different image, selected at random from
path_one_images to the user. After the user views three such images they are then presented with the previous three on the forth page. This is repeated until all 9 images are seen by the user. In the previous question I asked shortening the code proved to be a simple matter as each of my
if statements were essentially doing the same thing. However In this case they are further nested, undertake different (but similar) functions and are interrupted by the multi views. Has anyone got any suggestions for optimising, and preferably condensing the below code? Remember I am still learning so any suggestion would be helpful.
views.py
```
path_one_images = ['P1D1.jpg', 'P2D2.jpg', 'P3D3.jpg', 'P4D4.jpg', 'P5D5.jpg', 'P6D6.jpg', 'P7D7.jpg', 'P8D8.jpg', 'P9D9.jpg']
class SurveyWizardOne(SessionWizardView):
def get_context_data(self, form, **kwargs):
context = super(SurveyWizardOne, self).get_context_data(form, **kwargs)
if self.steps.current in ['5','6','7','8','9','10','11','12','13','14','15','16']:
print '\nThe available list of Path_One images is', path_one_images
global first_image, second_image, third_image, fourth_image, fifth_image, sixth_image, seventh_image, eight_image, ninth_image
if self.steps.current == '5':
first_image = random.choice(path_one_images)
path_one_images.remove(first_image)
display_image = first_image
Solution
Use
Avoid these kind of
Since
The last
Condensing
Instead of storing the image names in individual variables like
... and so on, you can follow the same logic for steps 13 - 16.
And you can probably improve your HTML template too,
to work with a list of images instead of hardcoded
if-elif-elseAvoid these kind of
if statements:if x == y:
# ...
if x == z: # note: z != y
# ...Since
x cannot be equal to two different values at the same time, you should use elif for the second and subsequent if statements on the same variable:if x == y:
# ...
elif x == z:
# ...
elif x == w:
# ...The last
elif is often just an else, if there can be no other possibility, for example here:if x in (1, 2, 3):
if x == 1:
# ...
elif x == 2:
# ...
else: # must be inevitably 3
# ...Condensing
Instead of storing the image names in individual variables like
first_image, second_image, ..., you should use an array, let's call it images. Then you can shorten a little bit:step = int(self.steps.current)
if step in (5, 6, 7):
images[step - 5] = image = random.choice(path_one_images)
path_one_images.remove(image)
context['display_image'] = image
elif step == 8:
context['first_image'] = images[0]
context['second_image'] = images[1]
context['third_image'] = images[2]
elif step in (9, 10, 11):
images[3 + step - 9] = image = random.choice(path_one_images)
path_one_images.remove(image)
context['display_image'] = image
elif step == 12:
context['fourth_image'] = images[3]
context['fifth_image'] = images[4]
context['sixth_image'] = images[5]... and so on, you can follow the same logic for steps 13 - 16.
And you can probably improve your HTML template too,
to work with a list of images instead of hardcoded
first_image, second_image, ..., and so on. Once you do that, you will be able to further simplify the Python part too, without the hardcoded context['first_image'], context['second_image'], ..., and so on.Code Snippets
if x == y:
# ...
if x == z: # note: z != y
# ...if x == y:
# ...
elif x == z:
# ...
elif x == w:
# ...if x in (1, 2, 3):
if x == 1:
# ...
elif x == 2:
# ...
else: # must be inevitably 3
# ...step = int(self.steps.current)
if step in (5, 6, 7):
images[step - 5] = image = random.choice(path_one_images)
path_one_images.remove(image)
context['display_image'] = image
elif step == 8:
context['first_image'] = images[0]
context['second_image'] = images[1]
context['third_image'] = images[2]
elif step in (9, 10, 11):
images[3 + step - 9] = image = random.choice(path_one_images)
path_one_images.remove(image)
context['display_image'] = image
elif step == 12:
context['fourth_image'] = images[3]
context['fifth_image'] = images[4]
context['sixth_image'] = images[5]Context
StackExchange Code Review Q#64451, answer score: 2
Revisions (0)
No revisions yet.