Optimisation of a views.py for survey application

Question

I have developed a small survey application in Django 1.6.2 which requires that the participants fill out a number of questions over 27 pages using a SessionWizardView.

Overview

The most unique feature of the application is that it allows the user to rate a selection of 9 images selected at random in isolation. The user then has to verify the rating they gave each image when they are re-shown them in groups of three.

At the beginning I have two instruction image rating tasks where I make it clear to the user what they have to do and two "Spike" questions to make sure that they are paying attention. Lastly I have two pages where all 9 images are shown again and they have to select their favorite and worst.

I implemented this using a get_context_data method of a SessionWizardView. Also included is my done method.

I previously posted a question about my models.py and there seems to be very few issues. I have also posted about my forms.py and am awaiting feedback. Its almost 400 lines of code (with logging comments so you might know what I was trying to do) but I thought it best to post it all.

I am completely self thought and have never programmed before and am just looking for feedback on how to improve my application. There is a lot of very obvious stuff I have no idea of.

views.py

class SurveyWizardOne(SessionWizardView):                             
    def get_context_data(self, form, **kwargs):
        context = super(SurveyWizardOne, self).get_context_data(form, **kwargs)                      
        step = int(self.steps.current)        

        if step == 0:
            self.request.session['path_one_images'] = ['P1D1.jpg', 'P2D2.jpg', 'P3D3.jpg', 'P4D4.jpg', 'P5D5.jpg', 'P6D6.jpg', 'P7D7.jpg', 'P8D8.jpg', 'P9D9.jpg']    
            self.request.session['instruction_task_one_images'] = ['IT1A.jpg', 'IT1B.jpg', 'IT1C.jpg']    
            self.request.session['instruction_task_two_images'] = ['IT2A.jpg', 'IT2B.jpg', 'IT2C.jpg']    
            self.request.session['images'] = []
            self.request.session['slider_DV_values'] = []

        PATH_ONE_IMAGES = self.request.session.get('path_one_images', [])        
        images = self.request.session.get('images', [])
        slider_DV_values = self.request.session.get('slider_DV_values', [])
        INSTRUCTION_TASK_ONE_IMAGES = self.request.session.get('instruction_task_one_images', [])
        INSTRUCTION_TASK_TWO_IMAGES = self.request.session.get('instruction_task_two_images', [])

        logger.debug('\n\nThis is the list of PATH_ONE_IMAGES at the START: %s', PATH_ONE_IMAGES)


        if step in range (0, 27):   
            self.request.session.update({
                                         'path_one_images': PATH_ONE_IMAGES,
                                         'images': images,
                                         'slider_DV_values': slider_DV_values,
                                         'instruction_task_one_images': INSTRUCTION_TASK_ONE_IMAGES,
                                         'instruction_task_two_images': INSTRUCTION_TASK_TWO_IMAGES
                                         })

            if step == 0:
                logger.debug('\n\nThis is your Instruction Task One page')
                instruction_task_first_image = random.choice(INSTRUCTION_TASK_ONE_IMAGES)     
                context['display_image'] = instruction_task_first_image                                 

            elif step == 1:                    
                logger.debug('\n\nThis is your Instruction Task Two page')
                instruction_task_second_image = random.choice(INSTRUCTION_TASK_TWO_IMAGES)   
                context['display_image'] = instruction_task_second_image  


            elif step == 9:    
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 9: %s', PATH_ONE_IMAGES)
                first_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(first_image)               
                context['display_image'] = first_image                                 
                images.insert(0, first_image)   
                self.request.session['first_image'] = images[0] 
                self.request.session.get('first_image')                          

                logger.debug('\n\nThis is your first image %s', images[0])
                logger.debug('\nThis is your images list in 9: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 9 %s', slider_DV_values)


            elif step == 10:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 10: %s', PATH_ONE_IMAGES)
                second_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(second_image)
                context['display_image'] = second_image                                 
                images.insert(1, second_image)   
                self.request.session['second_image'] = images[1] 
                self.request.session.get('second_image')    

                logger.debug('\n\nThis is your second image %s', images[1])
                logger.debug('\nThis is your images list in 10: %s', images)

                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(0, slider_value) 

                logger.debug('\n\n\nThis is your slider_DV_values in 10 %s', slider_DV_values)

            elif step == 11:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 11: %s', PATH_ONE_IMAGES)
                third_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(third_image)
                context['display_image'] = third_image                                 
                images.insert(2, third_image)                  
                self.request.session['third_image'] = images[2] 
                self.request.session.get('third_image')    

                logger.debug('\n\nThis is your third image %s', images[2])
                logger.debug('\nThis is your images list in 11: %s', images)

                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(1, slider_value)

                logger.debug('\n\n\nThis is your slider_DV_values in 11 %s', slider_DV_values)

            elif step == 12:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 12: %s', PATH_ONE_IMAGES)
                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(2, slider_value)

                logger.debug('\nThis is your images list in 12: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 12 %s', slider_DV_values)

                context.update({'first_image' : self.request.session['first_image'],
                                'second_image' : self.request.session['second_image'],
                                'third_image' : self.request.session['third_image'],                 
                                'first_slider' : slider_DV_values[0],
                                'second_slider' : slider_DV_values[1],
                                'third_slider' : slider_DV_values[2],})                               

            elif step == 13:
                #This is where I am trying to update my slider values on DV            
                slider_value1 = self.request.POST.get('slider_value1')
                slider_value2 = self.request.POST.get('slider_value2')
                slider_value3 = self.request.POST.get('slider_value3')

                if slider_value1 != '':
                    slider_DV_values.pop(0)                
                    slider_DV_values.insert(0, slider_value1)

                if slider_value2 != '':
                    slider_DV_values.pop(1)                
                    slider_DV_values.insert(1, slider_value2)

                if slider_value3 != '':
                    slider_DV_values.pop(2)                
                    slider_DV_values.insert(2, slider_value3)


                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 13: %s', PATH_ONE_IMAGES)
                fourth_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(fourth_image)               
                context['display_image'] = fourth_image                                 
                images.insert(3, fourth_image)   
                self.request.session['fourth_image'] = images[3] 
                self.request.session.get('fourth_image')                          

                logger.debug('\n\n\This is your fourth image %s', images[3])
                logger.debug('\nThis is your images list in 13: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 13 %s', slider_DV_values)

            elif step == 14:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 14: %s', PATH_ONE_IMAGES)
                fifth_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(fifth_image)
                context['display_image'] = fifth_image                                 
                images.insert(4, fifth_image)   
                self.request.session['fifth_image'] = images[4] 
                self.request.session.get('fifth_image')    

                logger.debug('\n\nThis is your fifth image %s', images[4])
                logger.debug('\nThis is your images list in 14: %s', images)

                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(3, slider_value) 

                logger.debug('\n\n\nThis is your slider_DV_values in 14 %s', slider_DV_values)

            elif step == 15:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 15: %s', PATH_ONE_IMAGES)
                sixth_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(sixth_image)
                context['display_image'] = sixth_image                                 
                images.insert(5, sixth_image)                  
                self.request.session['sixth_image'] = images[5] 
                self.request.session.get('sixth_image')    
                logger.debug('\n\nThis is your sixth image %s', images[5])
                logger.debug('\nThis is your images list in 15: %s', images)

                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(4, slider_value)

                logger.debug('\n\n\nThis is your slider_DV_values in 15 %s', slider_DV_values)

            elif step == 16:                 
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 16: %s', PATH_ONE_IMAGES)
                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:
                    slider_DV_values.insert(5, slider_value)  

                logger.debug('\nThis is your images list in 16: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 16 %s', slider_DV_values)

                context.update({'fourth_image' : self.request.session['fourth_image'],
                                'fifth_image' : self.request.session['fifth_image'],
                                'sixth_image' : self.request.session['sixth_image'],
                                'first_slider' : slider_DV_values[3],
                                'second_slider' : slider_DV_values[4],
                                'third_slider' : slider_DV_values[5],})     

            elif step == 17:                           
                #This is where I am trying to update my slider values on DV    
                #This is the spike 2 page
                slider_value1 = self.request.POST.get('slider_value1')
                slider_value2 = self.request.POST.get('slider_value2')
                slider_value3 = self.request.POST.get('slider_value3')

                if slider_value1 != '':
                    slider_DV_values.pop(3)                
                    slider_DV_values.insert(3, slider_value1)

                if slider_value2 != '':
                    slider_DV_values.pop(4)                
                    slider_DV_values.insert(4, slider_value2)

                if slider_value3 != '':
                    slider_DV_values.pop(5)                
                    slider_DV_values.insert(5, slider_value3)                                                      

            elif step == 18:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 18: %s', PATH_ONE_IMAGES)
                seventh_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(seventh_image)               
                context['display_image'] = seventh_image                                 
                images.insert(6, seventh_image)   
                self.request.session['seventh_image'] = images[6] 
                self.request.session.get('seventh_image')  

                logger.debug('\n\n\This is your seventh image %s', images[6])
                logger.debug('\nThis is your images list in 18: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 18 %s', slider_DV_values)

            elif step == 19:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 19: %s', PATH_ONE_IMAGES)
                eight_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(eight_image)
                context['display_image'] = eight_image                                 
                images.insert(7, eight_image)   
                self.request.session['eight_image'] = images[7] 
                self.request.session.get('eight_image')    

                logger.debug('\n\nThis is your eight image %s', images[7])
                logger.debug('\nThis is your images list in 19: %s', images)

                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(6, slider_value) 

                logger.debug('\n\n\nThis is your slider_DV_values in 19 %s', slider_DV_values)

            elif step == 20:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 20: %s', PATH_ONE_IMAGES)
                ninth_image = random.choice(PATH_ONE_IMAGES)   
                PATH_ONE_IMAGES.remove(ninth_image)
                context['display_image'] = ninth_image                                 
                images.insert(8, ninth_image)                  
                self.request.session['ninth_image'] = images[8] 
                self.request.session.get('ninth_image')    
                logger.debug('\n\nThis is your ninth image %s', images[8])
                logger.debug('\nThis is your images list in 20: %s', images)

                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:                
                    slider_DV_values.insert(7, slider_value)

                logger.debug('\n\n\nThis is your slider_DV_values in 20 %s', slider_DV_values)

            elif step == 21:                 
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 21: %s', PATH_ONE_IMAGES)
                slider_value = self.request.POST.get('slider_value')
                if slider_value is not None:
                    slider_DV_values.insert(8, slider_value)

                logger.debug('\nThis is your images list in 21: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 21 %s', slider_DV_values)

                context.update({'seventh_image' : self.request.session['seventh_image'],
                                'eight_image' : self.request.session['eight_image'],
                                'ninth_image' : self.request.session['ninth_image'],                 
                                'first_slider' : slider_DV_values[6],  
                                'second_slider' : slider_DV_values[7],
                                'third_slider' : slider_DV_values[8],}) 

            elif step == 22:
                #This is where I am trying to update my slider values on DV            
                slider_value1 = self.request.POST.get('slider_value1')
                slider_value2 = self.request.POST.get('slider_value2')
                slider_value3 = self.request.POST.get('slider_value3')

                if slider_value1 != '':
                    slider_DV_values.pop(6)                
                    slider_DV_values.insert(6, slider_value1)

                if slider_value2 != '':
                    slider_DV_values.pop(7)                
                    slider_DV_values.insert(7, slider_value2)

                if slider_value3 != '':
                    slider_DV_values.pop(8)                
                    slider_DV_values.insert(8, slider_value3)

                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 22: %s', PATH_ONE_IMAGES)
                logger.debug('\nThis is your images list in 22: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 22 %s', slider_DV_values)            

                context.update({'first_image' : images[0],
                           'second_image' : images[1],
                           'third_image' : images[2],
                           'fourth_image' : images[3],
                           'fifth_image' : images[4],
                           'sixth_image' : images[5],
                           'seventh_image' : images[6],
                           'eight_image' : images[7],
                           'ninth_image' : images[8], 
                           })

            elif step == 23:
                logger.debug('\n\nThis is the available list of PATH_ONE_IMAGES in 23: %s', PATH_ONE_IMAGES)
                logger.debug('\nThis is your images list in 21: %s', images)
                logger.debug('\n\n\nThis is your slider_DV_values in 23 %s', slider_DV_values)                 

                context.update({'first_image' : images[0],
                           'second_image' : images[1],
                           'third_image' : images[2],
                           'fourth_image' : images[3],
                           'fifth_image' : images[4],
                           'sixth_image' : images[5],
                           'seventh_image' : images[6],
                           'eight_image' : images[7],
                           'ninth_image' : images[8], 
                           })

            steps = ['9','10','11','13','14','15','18','19','20']              
            it_step_one = ['0']
            it_step_two = ['1']
            spike1 = ['8']
            spike2 = ['17']
            dv_steps = ['12','16','21',]  
            dv_nine_positive = ['22']       
            dv_nine_negative = ['23']       

            context.update({'steps' : steps,
                            'it_step_one' : it_step_one, 
                            'it_step_two' : it_step_two,
                            'spike1' : spike1,
                            'spike2' : spike2,
                            'dv_steps' : dv_steps, 
                            'dv_nine_positive' : dv_nine_positive, 
                            'dv_nine_negative' : dv_nine_negative,
                            })

        return context 


    def done(self, form_list, **kwargs):
        global SurveyWizardOneCounter
        global TotalMaxCounter 
        SurveyWizardOneCounter += 1   
        TotalMaxCounter += 1

        for form in form_list:
            form.save()

        return render(self.request, 'Return_to_AMT.html', {
            'form_data': [form.cleaned_data for form in form_list],            
        })  

Answer 1

Python specific

Docstrings

Documenting what your code does via some docstrings in your code can help a lot. This is especially relevant when other people are reading your code (like now!). In any case it's a habit that's worth getting into.

Checking for valid values

There are a few parts in your code where you do something like this:

if slider_value1 != '':

Presumably you are trying to check that slider_value1 has some sort of valid state before using it.

However, consider this code:

test = None

if test != '':
    print(test)
else:
    print("Invalid") 

What do you think it will do? I suggest you run this.

The more pythonic way of dealing with this is to do the following:

if slider_value:
    #do something

Refactoring

Long functions

Something that immediately stands out to me is the length of the function. Generally speaking you don't want to have any function be longer than say 50 lines of code, if you start getting really long functions you should consider refactoring into smaller functions. While long functions are not always bad they are frequently indicative of other problems.

Specifically you have this big chain of elif statements that are of the form step = .... Make those into their own functions.

Duplicated code

In this particular over-long function we can find a bunch of duplicated code. Generally speaking Python+Django has a philosophy called Don't repeat yourself

For example when you have:

if step == 0:
    logger.debug('\n\nThis is your Instruction Task One page')
    instruction_task_first_image = random.choice(INSTRUCTION_TASK_ONE_IMAGES)     
    context['display_image'] = instruction_task_first_image                                 

elif step == 1:                    
    logger.debug('\n\nThis is your Instruction Task Two page')
    instruction_task_second_image = random.choice(INSTRUCTION_TASK_TWO_IMAGES)   
    context['display_image'] = instruction_task_second_image  

You really seem to be doing just about the same thing here but using different variable names. Very similar things happen in other cases of the various handlers for step here. When you see that you are doing the same thing more than once but just changing names it's a fairly good indicator that you should create a function to do the task and pass in the relevant changes via parameters.

def do_step_1_2(task_name, image_choices, context):
    """Does step 1 and 2,  returns the appropriate context"""
    logger.debug('\n\nThis is your Instruction Task {0} page'.format(task_name))
    context['display_image'] = random.choice(image_choices)

You most likely would want to choose a better name than this. Now you can call them like so:

if step == 0:
    do_step_1_2("One", INSTRUCTION_TASK_ONE_IMAGES, context)
elif step == 1:
    do_step_1_2("Two", INSTRUCTION_TASK_TWO_IMAGES, context)

Suddenly we are starting to get less repeated code.

We can take this a bit further again by making a dictionary that calls off the different functions:

steps_dispatcher = {
    1 : do_step_1_2,
    2 : do_step_1_2,
}

step_names = {
    1 : "One",
    2 : "Two",
}

image_sets = {
    1 : INSTRUCTION_TASK_ONE_IMAGES,
    2 : INSTRUCTION_TASK_TWO_IMAGES.
}

try:
    steps_dispatcher[step](step_names[step], image_sets[step], context)
except KeyError:
    logger.debug("Invalid step!")

This might seem like a bit of overkill for just 2 cases but you have a whole bunch of cases here (survey steps I'm assuming) and I think the time spent on it would be very worthwhile. This is because it will force you to look at what code is shared and will let you consolidate that code into functions. The reduction in maintenance time in the future is usually the way in which this ROI of doing this will manifest itself. When you need to make a change you can now just make it in one place without having to first try to hunt down all the places it could have occurred (and it's really easy to miss stuff especially if you are in a hurry).

There's a fair bit more that can be improved here but I think this is a good start. Try to clean it up and feel free to post a follow up question.