# Optimising and condensing Django View Class

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 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
context['display_image'] = display_image

if self.steps.current == '6':
second_image = random.choice(path_one_images)
path_one_images.remove(second_image)
display_image = second_image
context['display_image'] = display_image

if self.steps.current == '7':
third_image = random.choice(path_one_images)
path_one_images.remove(third_image)
display_image = third_image
context['display_image'] = display_image

if self.steps.current == '8':
context['first_image'] = first_image
context['second_image'] = second_image
context['third_image'] = third_image

if self.steps.current == '9':
fourth_image = random.choice(path_one_images)
path_one_images.remove(fourth_image)
display_image = fourth_image
context['display_image'] = display_image

if self.steps.current == '10':
fifth_image = random.choice(path_one_images)
path_one_images.remove(fifth_image)
display_image = fifth_image
context['display_image'] = display_image

if self.steps.current == '11':
sixth_image = random.choice(path_one_images)
path_one_images.remove(sixth_image)
display_image = sixth_image
context['display_image'] = display_image

if self.steps.current == '12':
context['fourth_image'] = fourth_image
context['fifth_image'] = fifth_image
context['sixth_image'] = sixth_image

if self.steps.current == '13':
seventh_image = random.choice(path_one_images)
path_one_images.remove(seventh_image)
display_image = seventh_image
context['display_image'] = display_image

if self.steps.current == '14':
eight_image = random.choice(path_one_images)
path_one_images.remove(eight_image)
display_image = eight_image
context['display_image'] = display_image

if self.steps.current == '15':
ninth_image = random.choice(path_one_images)
path_one_images.remove(ninth_image)
display_image = ninth_image
context['display_image'] = display_image

if self.steps.current == '16':
context['seventh_image'] = seventh_image
context['eight_image'] = eight_image
context['ninth_image'] = ninth_image

steps = ['5','6','7','9','10','11','13','14','15']

context.update({'steps': steps})

return context


### Use if-elif-else

Avoid 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.

• Thanks for your answer. Its very helpfull. The only part I dont understand is images[step - 5] = image = random.choice(path_one_images) and images[3 + step - 9] = image = random.choice(path_one_images) Could you explain whats going on here please? – Deepend Oct 2 '14 at 14:56
• It's to derive the index in the images array based on the step number. For example: 5 => 0, 6 => 1, 7 => 2, and then 9 => 3, 10 => 4, and so on. – janos Oct 2 '14 at 15:37
• Hi Janos, I am having an issue with the above code. I thought I had it working but it is now giving me an error Exception Value: global name 'images' is not defined When i define images then the array wont update. Any ideas? Thanks – Deepend Oct 6 '14 at 17:17

### Code as is

Constants should use upper case notation. path_one_images should be PATH_ONE_IMAGES. If it is not constants it should be assigned from constant.

One more hard-coded constanst can be found there:

if self.steps.current in ['5','6','7','8','9','10','11','12','13','14','15','16']:


display_image used, but it have no sense.

                display_image = second_image
context['display_image'] = display_image


WOW, ...

global first_image, second_image, third_image, fourth_image, fifth_image,\
sixth_image, seventh_image, eight_image, ninth_image


... where did they come from?

### Proposals

IMHO: following blocks should not be part of view, it's can be some util that implements needed behaviour.

            if self.steps.current == '11':
sixth_image = random.choice(path_one_images)
path_one_images.remove(sixth_image)
display_image = sixth_image
context['display_image'] = display_image


At least this code is called more than once, so it is reasonable to make a helper function for this purpose and make more reusable code. But I'm sure that you need few strategy classes and multiform manager to hold options and build flow using strategy instances.