1

I'm refactoring some code that a friend wrote and recently stumbled across this function:

def setup_parameters(self, data):
    '''Parse raw data to determine game settings.'''
    for line in data.split('\n'):
      line = line.strip().lower()
      if line:
        tokens = line.split()

        self.L.debug("tokens: " + str(tokens))

        key = tokens[0]
        if key == 'cols':
          self.width = int(tokens[1])
        elif key == 'rows':
          self.height = int(tokens[1])
        elif key == 'player_seed':
          random.seed(int(tokens[1]))
        elif key == 'turntime':
          self.turntime = int(tokens[1])
        elif key == 'loadtime':
          self.loadtime = int(tokens[1])
        elif key == 'viewradius2':
          self.viewradius2 = int(tokens[1])
        elif key == 'attackradius2':
          self.attackradius2 = int(tokens[1])
        elif key == 'spawnradius2':
          self.spawnradius2 = int(tokens[1])

As you can see, there is a nasty kind of switch statement here, that clearly calls for a dictionary. I'm tempted to write this as a class dictionary since the keys are constant, but since the keys map to attributes of an instance (ie, 'cols': self.width) this doesn't compile.

My question is then, what is the right way to refactor such code?

2
  • Duplicate question: stackoverflow.com/questions/60208/…
    – JBernardo
    Commented Sep 9, 2011 at 19:02
  • 1
    Seeing that you are taking the effort of refactoring that code I suggest you take a look and use the standard python ConfigParser module
    – fabmilo
    Commented Sep 9, 2011 at 19:55

3 Answers 3

6

Map the keys to the names of the attributes, and use setattr(self, attribute_name, int(tokens[1]) to set the value. E.g.:

attribute_dict = dict(cols="width", rows="height", turntime="turntime", ...)
[...]
value = int(tokens[1])
if key == "player_seed":
    random.seed(value)
else:
    setattr(self, attribute_dict[key], value)
3
  • 1
    He has a call to random.seed(int(tokens[1])), so that won't work for all the cases. Commented Sep 9, 2011 at 19:03
  • That's why it is handled by a separate if branch :)
    – Tamás
    Commented Sep 9, 2011 at 20:36
  • Well that wasn't there until you edited it and put the code sample in :) Commented Sep 9, 2011 at 20:41
1

You can build up a dictionary with the keys as accessors and lambda functions to execute the code for each key.

1

setup a dict with actions like

actions = dict(cols = lambda tokens: setattr(self, "width", int(tokens[1]), ... 
               player_seed = lambda tokens: random.seed(int(tokens[1]))
              )

and then:

 actions[key](tokens)

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.