5

I have a function that looks something like this:

function_name(step, ... , typ):
    if typ == 'some type of calc method':
         if step == 1:
             do_me_at_step_1(...)
         elif step == 2:
             do_me_at_step_2(...)
         elif ...
    elif typ == 'another calc method':
         if step == 1:
             do_me_at_step_1(...)
         elif ...

Hopefully the general structure is clear here. What are the differences between the different typs and how they are handled? There is one instance of typ which changes one variable in a very different way from all the others. All instances of typ vary in the number of separate step instructions, ranging from an if step == 1: ... else: to if step == 1 ... elif step == 5 ... else. The last elif and the else statements are different, basically.

How can I better structure this function, because it currently looks horrific?

My thought was to pull out all the initial steps...but I have to have a way to associate each typ with the step which becomes "key". So I thought somewhere along the lines of:

function_name(step, ... , typ):
    orders = {typ1:key_step, typ2:key_step, ...}
    while key_step['typ1'] != step:
        if step == 1:
            ...
        step +=1

But I noticed two problems: one, this doesn't really fully take advantage of the while loop. It would require me to add if statements up until the max step out of all the typs that is not "key" in there, which doesn't seem to me to really take advantage of the information in orders.

The other problem is once you are out of the while loop. I suppose you could do something like

if typ == 'something':
    do_the_key_step_function_for_this_typ(...)

but this really isn't doing anything new to clean up this function. How can I change the structure of this code so it is cleaner and shorter? There's an underlying structure, so I feel like there should be a better way to do this.

Feel free to ask me if you need any specifics about this function, but hopefully there are enough details provided here. Thanks!

3 Answers 3

7

Your initial function executes one specific step for one specific type, both of them being provided as argument. In the second case you iterate over steps trying to find the right one.

Alternative 1: double dictionary

Based on your alternative, I propose you to consider a dictionary of dictionaries, using two indexes instead of just one. Exemple:

# just some functions to be invoked
def do_me_at_step_1():
    return 1

def do_me_at_step_2():
    return 2

def do_other_step_2():
    return 102

# Use a dictionary assigning one type to a second level dictionary   
# In the second level dictionary, assign one step with a function 
command = { 'A': { 1:do_me_at_step_1, 2:do_me_at_step_2 }, 'B': { 1:do_me_at_step_1, 2:do_other_step_2 } }

# How you'd invoke the right function in function_name()
print (command['A'][1]())
print (command['A'][2]())
print (command['B'][1]())
print (command['B'][2]())

So in this case, your function_name() just finds the function to invoke using a dictionary of dictionaries, and then invokes them.

Alternative 2: object oriented code ?

You could also consider using an object oriented approach:

So in fact, you'd create different classes, each corresponding to a different type. And then you'd invoke for an instantiation of one specific class the method corresponding to the step.

In fact, the implementation would be very close to the the state pattern

3
  • 1
    So the function would effectively look like function_name(): return command[typ][step](inputs to function)? That seems like a nice solution to me. But then you get a question at an earlier level: is there a better way of "generating" command? Considering that step 1 across all typs is going to be the same, and so forth up to the key step for each typ.
    – auden
    Jul 8, 2018 at 0:45
  • @heather yes: if most of the steps are similar, but with a couple of exception, the approach would be to use an object oriented structure: the common behavior is factored in the parent class, and the deviations/variants would override some methods (steps) in child classes. This technique is the "template method" template. State pattern is also very relevant, because each step is like a state.
    – Christophe
    Jul 8, 2018 at 0:53
  • Ah, I just noticed a problem with this code. Control is indexed both by typ and step, but this forgets that after a certain point, the same function is used. In other words, this means that I will need to type out an infinite number of steps corresponding to the same function for each typ to do the same thing the else statement is currently doing.
    – auden
    Jul 8, 2018 at 1:31
4

This is based on Christophe's answer.


Christophe's answer focuses on using a double dictionary, with one set of keys the typ and the other set of keys the step. However, this doesn't account for the else statement - that is, at a certain point, no matter the value of step, the same function is used, but the dictionary can't know that.

There's actually a rather simple way to account for this, and that's what I'm using right now (though I'd be very open to other answers that appear cleaner): use a second dictionary with the default cases. So the function becomes:

def function_name(step, ... , typ):
    command = { 'A': { 1:do_me_at_step_1, 2:do_me_at_step_2 }, 'B': { 1:do_me_at_step_1, 2:do_other_step_2 } }
    default_command = {'A':final_function_for_A, 'B':final_function_for_B, ...}
    try:
        return command[typ][step](inputs to function)
    except KeyError:
        return default_command[typ](inputs to function)

which is a lot nicer than what I had.


Edit: because catching an error is expensive, the last four lines would probably be better as an if-else:

if step in command[typ].keyValues():
    return command[typ][step](...)
else:
    return default_command[typ](...)
1
  • 1
    Nice extension of my proposal, which itself was inspired from your initial refactoring. A perfect example of team dynamics ;-)
    – Christophe
    Jul 8, 2018 at 8:41
1

You can call your typ functions in between your common steps if you pass the function as a parameter, then call it during your keystep.

# defining some dummy functions as the common code
def f1(*args):
    pass

def f2(*args):
    pass

def f3(*args):
    pass

# defining some functions for a specific 'typ'
def integer(*args):
    pass

def complex(*args):
    pass

# a dictionary that 1-indexes the common code
common_code = {
    1 : f1,
    2 : f2,
    3 : f3,
    # some lambdas in case your common code is simple or short
    4 : (lambda *args: args),
    5 : (lambda *args: args),
}

# the main function that lets us call a function for a chosen keystep
def calc_switch(step, calc_typ, *args):
    last = len(common_code)
    # perform the common steps from 1 until the chosen step
    # this for loop will not execute code if the user chooses 1
    for i in range(1, step):
        common_code[i](args)

    # the call to our step-dependent function
    calc_typ(args)

    # Continue calculating common code
    # if you want to exclude the step chosen by the user, then 
    # set the first argument of range to step+1
    for i in range(step, last + 1):
        common_code[i](args)


# some example calls, where the first argument is the key-step
calc_switch(2, complex)
calc_switch(4, integer)

Now you only have to define your key step functions once, and your common behavior once. Additional modification to the common code will be done in the common code dictionary.

Furthermore if you wanted to isolate to the call for a 'typ' you could use partial functions. For example:

from functools import partial

do_integer = partial(calc_switch, 2, complex)
do_complex = partial(calc_switch, 4, integer)

Now you will have a complete do_integer function that performs all common tasks, does the calculation for the typ 'integer' at the defined key-step, performs any common functions after that keystep, and takes any arguments and and passes it to the base function (via the *args parameter).

I hope this helps! The code is in Python 2.7, but the dictionary stuff and the partial application stuff is all in Python 3.

2
  • Interesting answer, thank you! (You may wish to change the variable name "max", as that's a built-in.)
    – auden
    Jul 8, 2018 at 2:32
  • Thanks you! I changed the variable max like you suggested. Jul 8, 2018 at 2:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.