current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Join the Stack Overflow Community
Stack Overflow is a community of 6.6 million programmers, just like you, helping each other.
Join them; it only takes a minute:
Sign up
up vote 13 down vote favorite

I have encountered a problem when putting all the modules I've developed into the main program. The switch dictionary I've created can be seen below:

def Tank_Shape_Calcs(Tank_Shape, level, area, dish, radius, length, Strapping_Table, Tank_Number):

    switcher = {
        0: vertical.Vertical_Tank(level, area),
        1: horiz.Horiz_Cylinder_Dished_Ends(dish, radius, level, length),
        2: strapping.Calc_Strapped_Volume(Strapping_Table, level),
        3: poly.Fifth_Poly_Calcs(Tank_Number)
    }
    return switcher.get(Tank_Shape, "ERROR: Tank type not valid")

The tank shape is set in the main file in a loop for each of the tanks. The first tank has Tank_Shape = 2 so I would expect it to execute the Calc_Strapped_Volume() function.

I have tried testing it, and the switcher function is definitely reading Tank_Shape as 2. Also if I change the functions to strings, it will print out the correct string.

The problem is that the functions seem to be executed sequentially, until the correct function is called. This results in errors as the data I'm using will only work with the correct function.

Is there a way to only execute the correct function?

share|improve this question
3  
The way you've structured your code (manually using a dictionary of lambdas for handling multiple behaviors, combining several different arguments that may or may not be needed depending on the desired behavior, &c.) suggests that you are new to Python. I would suggest you post a more complete copy of your code to codereview.stackexchange.com; some of the users there might be able to give you some tips on how to write your code more effectively :') – deinlebenandern 4 hours ago
up vote 27 down vote accepted

All your functions are executed when you build the dictionary, not when you access the key.

You need to use lambda (without any parameters, they are already known) to make sure the function is only called when required:

switcher = {
    0: lambda : vertical.Vertical_Tank(level, area),
    1: lambda : horiz.Horiz_Cylinder_Dished_Ends(dish, radius, level, length),
    2: lambda : strapping.Calc_Strapped_Volume(Strapping_Table, level),
    3: lambda : poly.Fifth_Poly_Calcs(Tank_Number)
}

then call when you return, with the error message as a lambda that returns it:

return switcher.get(Tank_Shape, lambda : "ERROR: Tank type not valid")()
share|improve this answer
7  
or functools.partial instead of lambda – volcano 12 hours ago
    
didn't know about functools.partial. Duly noted. (I hope to reuse it in a future answer :)) However, in that case lambda seems to be the best choice, since all args are passed, not only some.. – Jean-François Fabre 12 hours ago
2  
Always happy to be of service :-) , partial is excellent for callbacks. I myself sin with lambda often enough, but - IMHO, in this case partial looks a bit cleaner. Matter of taste. – volcano 11 hours ago
    
How the heck do you have 29k rep and a 999 score in Python and not know about functools.partial? – jpmc26 11 hours ago
3  
@jpmc26 I suppose you're teasing me in a friendly way. Well 29k doesn't come all from python of course. and I confess I don't know everything in Python. Martijn is that guy probably. – Jean-François Fabre 11 hours ago
up vote 14 down vote

As noted, the functions will get invoked during dictionary construction. Besides that, there's two other issues I see here:

  • Redefining switcher during every invocation of the function Tank_Shape_Calcs, this generally isn't a good idea.
  • Requiring all arguments to be passed (due to their definition as positionals) when only a handful of them might be needed, this is why we have *args :-)

If my understanding of what you're up to is correct, I'd move switcher outside the function, as a Tank_Shape to function object mapping:

switcher = {
    0: vertical.Vertical_Tank,
    1: horiz.Horiz_Cylinder_Dished_Ends,
    2: strapping.Calc_Strapped_Volume,
    3: poly.Fifth_Poly_Calcs
}

Then, define Tank_Shape_Calcs to take the excess arguments as a tuple with *args:

def Tank_Shape_Calcs(Tank_Shape, *args):
    return switcher.get(Tank_Shape, lambda *_: "ERROR: Tank type not valid")(*args)

and invoke your function after .getting it.

This also plays off Jean's trick to define a lambda in .get but does so with *_ in order to allow it to get called with many args (which are consequently ignored).

share|improve this answer
2  
just beautiful ! – Jean-François Fabre 11 hours ago
1  
@Jean-FrançoisFabre I'm still looking over it fearing I missed something, the fact that no minimal example which I can test is given is feeding my insecurities. – Jim Fasarakis-Hilliard 11 hours ago
1  
The problem, that I see, is that the single functions in the dictionary do not take all arguments from the outer function... as the result he will need to modify them, so they can handle all the arguments, even if they are not used in the particular function – BloodyD 11 hours ago
2  
Neat solution, but It seems a shame to use a dict when the keys are just like indexes, why not make switcher a list? – Chris_Rands 11 hours ago
2  
Dunno what the OP is exactly up to @Chris_Rands but a dict here allows for easy handling of the case where a key that doesn't exist via .get. You could do it with a list but it would arguably look clunkier. Also, a dictionary is intuitively more applicable here as a simulation of the switch statement . – Jim Fasarakis-Hilliard 11 hours ago
up vote 6 down vote

What you are doing in your code, is creating a dictionary with integer keys (0-3) and function results as values. Hence, you first call all the functions and then access the return values of these functions. I would change your code as following:

def Tank_Shape_Calcs(Tank_Shape, level, area, dish, radius, length, Strapping_Table, Tank_Number):
    switcher = {
        0: (vertical.Vertical_Tank, (level, area)),
        1: (horiz.Horiz_Cylinder_Dished_Ends, (dish, radius, level, length)),
        2: (strapping.Calc_Strapped_Volume, (Strapping_Table, level)),
        3: (poly.Fifth_Poly_Calcs, (Tank_Number,))
    }
    func, args = switcher.get(Tank_Shape, (None, None))
    if func is not None: 
        return func(*args)

Here, you get first the function you want to call with the according arguments and call it.

share|improve this answer
    
That's a good idea. You didn't emulate the error message, though, so if Tank_Shape is > 3 it crashes instead of returning a clear error string. – Jean-François Fabre 12 hours ago
    
yes, you are right, I've inserted a check – BloodyD 11 hours ago
1  
To me this feels like the most elegant solution proposed so far. – SethMMorton 9 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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