Re-thinking my if-thens - a software engineering question

[Bruno Desthuilliers]

metaperl a écrit :
> Ok, I have a module called textgen.py. The point of this module is to
> generate a csv file from an array of dictionaries.

Err... You know there's a csv module in the stdlib, don't you ?

> As I iterate through
> each dictionary, I "massage" the dictionary values before writing them
> out to csv. Now, for one dictionary entry, I have the following code:
> 
>             if dict_key == 'PCN':
> 		fields = dict_val.split("/")
>                 mo = re.match( '(\w{2})(\d{2})(\d{2})' , fields[1] )
> 		if mo:
> 		    dict_val = "%s/%s%s/%s" % (fields[0], mo.group(1), mo.group(3),
> fields[2][1:])
> 		else:
> 		    dict_val = dict_val

FWIW, you could as well skip this else clause

> Ok, so now here is what has happened. This code was based on the
> assumption that dict_val would have 2 forward slashes in it. It turns
> out that I need to follow a different process of massaging when no
> slashes are present. A naive solution would be something like:
> 
> if dict_key == 'PCN':
>     fields = dict_val.split("/")
>     if fields == 3:
       if len(fields) == 3:
>         dict_val = pcn_three(fields) # where pcn_three
> is the code above
>      else:
>          # new logic
> 
> But I am wondering if I should abstract the flow of control into a
> class or something.

Depends... If your code is starting to smell, then yes, it's probably 
time to refactor a bit. Note that you don't necessarily needs OO to 
clean up a mess. Python functions are objects on their own, so you can 
use them like any other variable. A common idiom is to use a dict as a 
dispatch table:

def test1(arg):
   print "test1", arg

def test2(arg):
   print "test2", arg

def test3(arg):
   print "test3", arg

dispatch = {
   'TEST_ONE': test1,
   'TEST_TWO': test2,
   'TEST_THREE': test3,
}
default = lambda arg: arg

#...

func = dispatch(dict_key)
dict_val = func(fields)

And using lambdas you can even come close to Perl !-)

dispatch = {
   'PCN' : lambda arg: \
           (new_logic, pcn_three)[arg.count('/') == 2](arg.split('/')),
   'XXX' : code_here,
   ...
}

Or you could write some "dispatcher helper" functions, like:

def dispatch_on_arg_count(default, *funcs):
   def _dispatcher(*args):
     try:
       func = funcs[len(args)]
     except IndexError:
       func =default
     return func(*args)
   return _dispatcher

dispatch = {
   'PCN': dispatch_on_arg_count(
            default,
            default,
            pcn_one,
            pcn_two,
            pcn_three
          ),
    'XXX' : code_here,
}


Or you could check Philip Eby's dispatch package...

And of course, since Python classes are themselves callables (no 'new' 
operator, you just call the class to instanciate it), and instances can 
be callable too (just define a __call__(self, ...) method), you can 
throw OO in the mix too  !-)

Now weither it will make your code more readable is another question...

> Ideas welcome.
> 

HTH