Refactoring huge if..elif in init how?

Question

I have this huge __init__ method which I want to clean up even more after watching Clean Coders videos (content of Clean Code book by Robert Cecil Martin).

How could I get refactor the gigantic if..elif block in a nice, readable way? Or should I leave it as it is? Do you find it readable?

@total_ordering
class Range:
    """Parses a poker range (str) into tuple of Combos (or Hands)."""

    def __init__(self, range=''):
        self._pairs = set()
        self._suiteds = set()
        self._offsuits = set()

        for token, value in _RegexRangeLexer(range):
            if token == 'ALL':
                for card in itertools.combinations('AKQJT98765432', 2):
                    self._add_offsuit(card)
                    self._add_suited(card)
                for rank in 'AKQJT98765432':
                    self._add_pair(rank)

                # full range, no need to parse any more token
                break

            elif token == 'PAIR':
                self._add_pair(value)

            elif token == 'PAIR_PLUS':
                smallest = Rank(value)
                for rank in (rank.value for rank in Rank if rank >= smallest):
                    self._add_pair(rank)

            elif token == 'PAIR_MINUS':
                biggest = Rank(value)
                for rank in (rank.value for rank in Rank if rank <= biggest):
                    self._add_pair(rank)

            elif token == 'PAIR_DASH':
                first, second = Rank(value[0]), Rank(value[1])
                ranks = (rank.value for rank in Rank if first <= rank <= second)
                for rank in ranks:
                    self._add_pair(rank)

            elif token == 'BOTH':
                self._add_offsuit(value[0] + value[1])
                self._add_suited(value[0] + value[1])

            elif token == 'X_BOTH':
                for rank in (r.value for r in Rank if r < Rank(value)):
                    self._add_suited(value + rank)
                    self._add_offsuit(value + rank)

            elif token == 'OFFSUIT':
                self._add_offsuit(value[0] + value[1])

            elif token == 'SUITED':
                self._add_suited(value[0] + value[1])

            elif token == 'X_OFFSUIT':
                biggest = Rank(value)
                for rank in (rank.value for rank in Rank if rank < biggest):
                    self._add_offsuit(value + rank)

            elif token == 'X_SUITED':
                biggest = Rank(value)
                for rank in (rank.value for rank in Rank if rank < biggest):
                    self._add_suited(value + rank)

            elif token == 'BOTH_PLUS':
                smaller, bigger = Rank(value[0]), Rank(value[1])
                for rank in (rank.value for rank in Rank if smaller <= rank < bigger):
                    self._add_suited(value[1] + rank)
                    self._add_offsuit(value[1] + rank)

            elif token == 'BOTH_MINUS':
                smaller, bigger = Rank(value[0]), Rank(value[1])
                for rank in (rank.value for rank in Rank if rank <= smaller):
                    self._add_suited(value[1] + rank)
                    self._add_offsuit(value[1] + rank)

            elif token in ('X_PLUS', 'X_SUITED_PLUS', 'X_OFFSUIT_PLUS'):
                smallest = Rank(value)
                first_ranks = (rank for rank in Rank if rank >= smallest)

                for rank1 in first_ranks:
                    second_ranks = (rank for rank in Rank if rank < rank1)
                    for rank2 in second_ranks:
                        if token != 'X_OFFSUIT_PLUS':
                            self._add_suited(rank1.value + rank2.value)
                        if token != 'X_SUITED_PLUS':
                            self._add_offsuit(rank1.value + rank2.value)

            elif token in ('X_MINUS', 'X_SUITED_MINUS', 'X_OFFSUIT_MINUS'):
                biggest = Rank(value)
                first_ranks = (rank for rank in Rank if rank <= biggest)

                for rank1 in first_ranks:
                    second_ranks = (rank for rank in Rank if rank < rank1)
                    for rank2 in second_ranks:
                        if token != 'X_OFFSUIT_MINUS':
                            self._add_suited(rank1.value + rank2.value)
                        if token != 'X_SUITED_MINUS':
                            self._add_offsuit(rank1.value + rank2.value)

            elif token == 'COMBO':
                combo = Combo(value)
                if combo.is_pair:
                    self._pairs.add(combo)
                elif combo.is_suited:
                    self._suiteds.add(combo)
                else:
                    self._offsuits.add(combo)

            elif token == 'OFFSUIT_PLUS':
                smaller, bigger = Rank(value[0]), Rank(value[1])
                for rank in (rank.value for rank in Rank if smaller <= rank < bigger):
                    self._add_offsuit(value[1] + rank)

            elif token == 'OFFSUIT_MINUS':
                smaller, bigger = Rank(value[0]), Rank(value[1])
                for rank in (rank.value for rank in Rank if rank <= smaller):
                    self._add_offsuit(value[1] + rank)

            elif token == 'SUITED_PLUS':
                smaller, bigger = Rank(value[0]), Rank(value[1])
                for rank in (rank.value for rank in Rank if smaller <= rank < bigger):
                    self._add_suited(value[1] + rank)

            elif token == 'SUITED_MINUS':
                smaller, bigger = Rank(value[0]), Rank(value[1])
                for rank in (rank.value for rank in Rank if rank <= smaller):
                    self._add_suited(value[1] + rank)

            elif token == 'BOTH_DASH':
                smaller, bigger = Rank(value[1]), Rank(value[2])
                for rank in (rank.value for rank in Rank if smaller <= rank <= bigger):
                    self._add_offsuit(value[0] + rank)
                    self._add_suited(value[0] + rank)

            elif token == 'OFFSUIT_DASH':
                smaller, bigger = Rank(value[1]), Rank(value[2])
                for rank in (rank.value for rank in Rank if smaller <= rank <= bigger):
                    self._add_offsuit(value[0] + rank)

            elif token == 'SUITED_DASH':
                smaller, bigger = Rank(value[1]), Rank(value[2])
                for rank in (rank.value for rank in Rank if smaller <= rank <= bigger):
                    self._add_suited(value[0] + rank)

There is a similar question: Refactor deeply nested if-else, but my conditions cannot be simplified any more (well possibly except the token in ('X_PLUS', 'X_SUITED_PLUS', 'X_OFFSUIT_PLUS') parts could be flatten out).

Maybe I could wrap small functions, but the if..elif part would still be huge. Could you show a real life example how these vast conditions are simplified usually?

Dororo · Accepted Answer · 2014-08-22 19:25:25Z

3

Typically massive if elifs are equivalent to a switch statement. Python doesn't have switch statements and instead relies on dicts.

I would have a dictionary of str: function, which would break up the huge method and avoid the huge if elifs.

As an aside, many of these blocks are very closely related, often only the list in the for loop is different (xxx_plus and xxx_minus). It would be nice if that could be factored out into a single method, with an arg to say how the list should be created.

edited Aug 22, 2014 at 19:25

answered Aug 22, 2014 at 19:18

Dororo

1313 bronze badges

Add a comment |

Emanuele Paolini · Accepted Answer · 2014-08-22 20:16:40Z

For each token you are describing a triple of sets (pairs,suited,off_suite) which are parameterized over a value.

It would be nice to have a code like this in your __init__:

    for token, value in _RegexRangeLexer(range):
        tok = tokens[token](value)
        self._add_pair(tok.pair)
        self._add_suited(tok.suited)
        self._add_off_suite(tok.off_suite)

which can be achieved if you define a dictionary of classes. The code could become even longer, but separates more clearly the logic of your Range class by the information about different Tokens. If these Tokens have other functionality using a class becomes even more convenient.

The definition of token is clearly separated and does not interfere in the logic of the application. Also, adding a new token is a matter of defining a new class and registering it in the dictionary of tokens. Similar token could benefit in inheritance. For example:

class TokenX(object):
    kind = None # define in derived classes    

    def __init__(self,value):
        setattr(self,self.kind,value)

class TokenXOffSuit(TokenX):
    kind = 'off_suit'

class TokenXSuited(TokenX):
    kind = 'suited'

Wow, I did not even thought about separating the token definitions! I already separated the lexer, because at the first time I was doing parsing also, so even the parser logic was in the __init__ :) but this should clear things out even more! Thanks. — kissgyorgy, Commented Aug 22, 2014 at 21:46

georg · Accepted Answer · 2014-08-23 17:30:17Z

The usual answer to this is dynamic methods:

def __init__(self, range=''):
    ...
    for token, value in _RegexRangeLexer(range):
        try:
            fun = getattr(self, 'token_' + token.lower())
        except AttributeError:
            # no such token! do something...

        fun(value)

Basically, this generates a method name from the token and attempts to call it. Then, define handler methods like:

def token_all(self, value):
    for card in itertools.combinations('AKQJT98765432', 2):
        self._add_offsuit(card)
        self._add_suited(card)
    for rank in 'AKQJT98765432':
        self._add_pair(rank)

def token_pair(self, value):
    # etc
    pass

This keeps your main loop tidy and allows you to add new token types easily.

As a side note, range is a poor choice for a variable name because it masks the builtin with the same name.

barak manos · Accepted Answer · 2014-08-23 19:20:43Z

For each token, define a function which takes a Range instance and a value.

Then, define a dictionary which maps each token to the corresponding function.

Finally, iterate the tokens and invoke each of the functions through the dictionary.

class Range:

    def ALL(range,value):
        for card in itertools.combinations('AKQJT98765432', 2):
            range._add_offsuit(card)
            range._add_suited(card)
        for rank in 'AKQJT98765432':
            range._add_pair(rank)

    def PAIR(range,value):
        range._add_pair(value)

    def PAIR_PLUS(range,value):
        smallest = Rank(value)
        for rank in (rank.value for rank in Rank if rank >= smallest):
            range._add_pair(rank)

    def PAIR_MINUS(range,value):
        biggest = Rank(value)
        for rank in (rank.value for rank in Rank if rank <= biggest):
            range._add_pair(rank)

    def PAIR_DASH(range,value):
        first, second = Rank(value[0]), Rank(value[1])
        ranks = (rank.value for rank in Rank if first <= rank <= second)
        for rank in ranks:
            range._add_pair(rank)

    def BOTH(range,value):
        range._add_offsuit(value[0] + value[1])
        range._add_suited(value[0] + value[1])

    def X_BOTH(range,value):
        for rank in (r.value for r in Rank if r < Rank(value)):
            range._add_suited(value + rank)
            range._add_offsuit(value + rank)

    def OFFSUIT(range,value):
        range._add_offsuit(value[0] + value[1])

    def SUITED(range,value):
        range._add_suited(value[0] + value[1])

    def X_OFFSUIT(range,value):
        biggest = Rank(value)
        for rank in (rank.value for rank in Rank if rank < biggest):
            range._add_offsuit(value + rank)

    def X_SUITED(range,value):
        biggest = Rank(value)
        for rank in (rank.value for rank in Rank if rank < biggest):
            range._add_suited(value + rank)

    def BOTH_PLUS(range,value):
        smaller, bigger = Rank(value[0]), Rank(value[1])
        for rank in (rank.value for rank in Rank if smaller <= rank < bigger):
            range._add_suited(value[1] + rank)
            range._add_offsuit(value[1] + rank)

    def BOTH_MINUS(range,value):
        smaller, bigger = Rank(value[0]), Rank(value[1])
        for rank in (rank.value for rank in Rank if rank <= smaller):
            range._add_suited(value[1] + rank)
            range._add_offsuit(value[1] + rank)

    def X_PLUS(range,value):
        smallest = Rank(value)
        first_ranks = (rank for rank in Rank if rank >= smallest)
        for rank1 in first_ranks:
            second_ranks = (rank for rank in Rank if rank < rank1)
            for rank2 in second_ranks:
                range._add_suited(rank1.value + rank2.value)
                range._add_offsuit(rank1.value + rank2.value)

    def X_SUITED_PLUS(range,value):
        smallest = Rank(value)
        first_ranks = (rank for rank in Rank if rank >= smallest)
        for rank1 in first_ranks:
            second_ranks = (rank for rank in Rank if rank < rank1)
            for rank2 in second_ranks:
                range._add_suited(rank1.value + rank2.value)

    def X_OFFSUIT_PLUS(range,value):
        smallest = Rank(value)
        first_ranks = (rank for rank in Rank if rank >= smallest)
        for rank1 in first_ranks:
            second_ranks = (rank for rank in Rank if rank < rank1)
            for rank2 in second_ranks:
                range._add_offsuit(rank1.value + rank2.value)

    def X_MINUS(range,value):
        biggest = Rank(value)
        first_ranks = (rank for rank in Rank if rank <= biggest)
        for rank1 in first_ranks:
            second_ranks = (rank for rank in Rank if rank < rank1)
            for rank2 in second_ranks:
                range._add_suited(rank1.value + rank2.value)
                range._add_offsuit(rank1.value + rank2.value)

    def X_SUITED_MINUS(range,value):
        biggest = Rank(value)
        first_ranks = (rank for rank in Rank if rank <= biggest)
        for rank1 in first_ranks:
            second_ranks = (rank for rank in Rank if rank < rank1)
            for rank2 in second_ranks:
                range._add_suited(rank1.value + rank2.value)

    def X_OFFSUIT_MINUS(range,value):
        biggest = Rank(value)
        first_ranks = (rank for rank in Rank if rank <= biggest)
        for rank1 in first_ranks:
            second_ranks = (rank for rank in Rank if rank < rank1)
            for rank2 in second_ranks:
                range._add_offsuit(rank1.value + rank2.value)

    def COMBO(range,value):
        combo = Combo(value)
        if combo.is_pair:
            range._pairs.add(combo)
        elif combo.is_suited:
            range._suiteds.add(combo)
        else:
            range._offsuits.add(combo)

    def OFFSUIT_PLUS(range,value):
        smaller, bigger = Rank(value[0]), Rank(value[1])
        for rank in (rank.value for rank in Rank if smaller <= rank < bigger):
            range._add_offsuit(value[1] + rank)

    def OFFSUIT_MINUS(range,value):
        smaller, bigger = Rank(value[0]), Rank(value[1])
        for rank in (rank.value for rank in Rank if rank <= smaller):
            range._add_offsuit(value[1] + rank)

    def SUITED_PLUS(range,value):
        smaller, bigger = Rank(value[0]), Rank(value[1])
        for rank in (rank.value for rank in Rank if smaller <= rank < bigger):
            range._add_suited(value[1] + rank)

    def SUITED_MINUS(range,value):
        smaller, bigger = Rank(value[0]), Rank(value[1])
        for rank in (rank.value for rank in Rank if rank <= smaller):
            range._add_suited(value[1] + rank)

    def BOTH_DASH(range,value):
        smaller, bigger = Rank(value[1]), Rank(value[2])
        for rank in (rank.value for rank in Rank if smaller <= rank <= bigger):
            range._add_offsuit(value[0] + rank)
            range._add_suited(value[0] + rank)

    def OFFSUIT_DASH(range,value):
        smaller, bigger = Rank(value[1]), Rank(value[2])
        for rank in (rank.value for rank in Rank if smaller <= rank <= bigger):
            range._add_offsuit(value[0] + rank)

    def SUITED_DASH(range,value):
        smaller, bigger = Rank(value[1]), Rank(value[2])
        for rank in (rank.value for rank in Rank if smaller <= rank <= bigger):
            range._add_suited(value[0] + rank)

    func_dict = {
        'ALL'             : ALL            ,
        'PAIR'            : PAIR           ,
        'PAIR_PLUS'       : PAIR_PLUS      ,
        'PAIR_MINUS'      : PAIR_MINUS     ,
        'PAIR_DASH'       : PAIR_DASH      ,
        'BOTH'            : BOTH           ,
        'X_BOTH'          : X_BOTH         ,
        'OFFSUIT'         : OFFSUIT        ,
        'SUITED'          : SUITED         ,
        'X_OFFSUIT'       : X_OFFSUIT      ,
        'X_SUITED'        : X_SUITED       ,
        'BOTH_PLUS'       : BOTH_PLUS      ,
        'BOTH_MINUS'      : BOTH_MINUS     ,
        'X_PLUS'          : X_PLUS         ,
        'X_SUITED_PLUS'   : X_SUITED_PLUS  ,
        'X_OFFSUIT_PLUS'  : X_OFFSUIT_PLUS ,
        'X_MINUS'         : X_MINUS        ,
        'X_SUITED_MINUS'  : X_SUITED_MINUS ,
        'X_OFFSUIT_MINUS' : X_OFFSUIT_MINUS,
        'COMBO'           : COMBO          ,
        'OFFSUIT_PLUS'    : OFFSUIT_PLUS   ,
        'OFFSUIT_MINUS'   : OFFSUIT_MINUS  ,
        'SUITED_PLUS'     : SUITED_PLUS    ,
        'SUITED_MINUS'    : SUITED_MINUS   ,
        'BOTH_DASH'       : BOTH_DASH      ,
        'OFFSUIT_DASH'    : OFFSUIT_DASH   ,
        'SUITED_DASH'     : SUITED_DASH    ,
    }        

    def __init__(self,range=''):
        self._pairs = set()
        self._suiteds = set()
        self._offsuits = set()
        for token, value in _RegexRangeLexer(range):
            if token in Range.func_dict:
                Range.func_dict[token](self,value)

This is not DRY. Also it's better to define those functions as methods on the Range class instead for obvious reasons. — kissgyorgy, Commented Aug 23, 2014 at 19:16
@Walkman: I have defined them in the Range class, but as class-methods rather than instance-methods (i.e., they don't have a self). — barak manos, Commented Aug 23, 2014 at 19:27

Stack Exchange Network

Refactoring huge if..elif in init how?

4 Answers 4

Your Answer

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

Linked

Hot Network Questions

Refactoring huge if..elif in __init__ how?

4 Answers 4

Your Answer

Sign up or log in

Post as a guest

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

Linked

Related

Hot Network Questions

Refactoring huge if..elif in init how?

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