Refactoring Python Code when the only thing changes are methods?

Question

Need some advice on how to refactoring this kind of code, as you can see, the basic code for all, left, right is the same, all what is changing is .strip(), .lstrip(), .rstrip(), is it possible to refactor this in a "beautiful" manner?

def clean_whitespaces(input, mode='all', ):
    result = None
    modes = (None, 'all', 'left', 'right')
    if mode not in modes:
        raise ValueError('clean_whitespaces: mode must be one of {}.'.format(modes))

    if mode == None:
        mode = 'all'

    if mode == 'all':
        if isinstance(input, str):
            result = input.strip()
        else:
            input = list(input)
            result = [entry.strip() for entry in input]
        return result

    elif mode == 'left':
        if isinstance(input, str):
            result = input.lstrip()
        else:
            input = list(input)
            result = [entry.lstrip() for entry in input]
        return result

    elif mode == 'right':
        if isinstance(input, str):
            result = input.rstrip()
        else:
            input = list(input)
            result = [entry.rstrip() for entry in input]
        return result

using dictionaries possibly, also input really shouldn't be used as a name for anything, it is a built-in — Matiiss
– Matiiss, Commented Nov 3, 2021 at 14:14
Agree with the use of dictionaries above. Your options for the values of the dictionary would either be the methods themselves (e.g. entry.strip) or the names of them (e.g. 'strip') which can then be looked up using getattr. — alani
– alani, Commented Nov 3, 2021 at 14:20
well both answers below are "beautiful" on its own, hard to decide what kind of style (dict or lambda), is there any difference in kind of "do's and don't" — Lecker Spam
– Lecker Spam, Commented Nov 3, 2021 at 14:45
in the lambda option you still iterate 3 times, by using 'getattr' you dont need to iterate over and over again — Bar Gans
– Bar Gans, Commented Nov 3, 2021 at 14:53

Bar Gans · Accepted Answer · 2021-11-04 08:43:14Z

4

You can use a dict for this:

modes_to_func = {'all': str.strip, 'left': str.lstrip, 'right': str.rstrip}

This way, you can avoid iterate over the modes:

def clean_whitespaces(input, mode='all', ):
    modes = (None, 'all', 'left', 'right')
    modes_to_func = {'all': str.strip, 'left': str.lstrip, 'right': str.rstrip}
    if mode not in modes:
        raise ValueError('clean_whitespaces: mode must be one of {}.'.format(modes))

    if mode is None:
        mode = 'all'

    if isinstance(input, str):
        result = modes_to_func[mode](input)

    else:
        input = list(input)
        result = [modes_to_func[mode](entry) for entry in input]
    return result

edited Nov 4, 2021 at 8:43

answered Nov 3, 2021 at 14:15

Bar Gans

1,6832 gold badges13 silver badges26 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Matiiss · Accepted Answer · 2021-11-03 17:03:21Z

So apparently you can do sth like str.strip(string_to_strip) (kinda makes sense because you simply pass in the instance (self)) which allows for this (with a few test cases at the end):

def clean_whitespaces(data, mode='all'):
    modes = {None: str.strip, 'all': str.strip,
             'left': str.lstrip, 'right': str.rstrip}
    if mode not in modes:
        raise ValueError(f'clean_whitespaces: mode must be one of '
                         f'{", ".join(map(repr, modes.keys()))}.')

    if isinstance(data, str):
        result = modes[mode](data)
    else:
        result = [modes[mode](str(entry)) for entry in list(data)]
    return result


case = '                      super long spaces before                           '
correct_cases = [
    'super long spaces before',
    'super long spaces before',
    'super long spaces before                           ',
    '                      super long spaces before'

]
for m, c in zip((None, 'all', 'left', 'right'), correct_cases):
    assert clean_whitespaces(case, 'w') == c
print('passed assertion')

Note: for some reason PyCharm doesn't like this because of some unexpected argument although doing it directly doesn't raise that warning

itprorh66 · Accepted Answer · 2021-11-03 18:49:19Z

1

I would use a dictionary and move all redundant code out of sub function and into main function.

def clean_whitespace(inp, mode='all'):

    def allSelected(x):
        return x.strip()

    def rightSelected(x):
        return x.rstrip()        
   
    def leftSelected(x):
        return x.lstrip()

    mdict = {'all': allSelected , 'left': leftSelected, 'right': rightSelected} 
    if mode == None:
        mode = 'all'
    try:
        if isinstance(inp, str):
            return mdict[mode](inp)
        else:
            return [mdict[mode](entry) for entry in inp]
    except KeyError:
        print('clean_whitespaces: mode must be one of {}.'.format([mdict.keys()]))

edited Nov 3, 2021 at 18:49

answered Nov 3, 2021 at 15:00

itprorh66

3,2784 gold badges13 silver badges26 bronze badges

2 Comments

Lecker Spam Over a year ago

there is a typo in def leftSelected(inp): return x.lstrip()inp should be x right?

itprorh66 Over a year ago

Updated, thanks for the catch

alex_noname · Accepted Answer · 2021-11-03 14:18:48Z

You can move the duplicate code into a function and customize it using a lambda:

def strip(strip_foo, input):
    if isinstance(input, str):
        result = strip_foo(input)
    else:
        input = list(input)
        result = [strip_foo(entry) for entry in input]
    return result


def clean_whitespaces(input, mode='all', ):
    result = None
    modes = (None, 'all', 'left', 'right')
    if mode not in modes:
        raise ValueError('clean_whitespaces: mode must be one of {}.'.format(modes))

    if mode == None:
        mode = 'all'

    if mode == 'all':
        return strip(lambda v: v.strip(), input)
    elif mode == 'left':
        return strip(lambda v: v.lstrip(), input)
    elif mode == 'right':
        return strip(lambda v: v.rstrip(), input)

Lecker Spam · Accepted Answer · 2021-11-03 16:42:55Z

0

So after reviewing all of the answers i came up with these, its more or less a merge of the 3 answers with dicts, thanks all of you for your hints (like input)

def clean_whitespaces(data, mode='all', ):
result = None
modes = {
    'all': str.strip,
    'left': str.lstrip,
    'right': str.rstrip
    }

if mode not in modes:
    raise ValueError('clean_whitespaces: mode must be one of {}.'.format([key for key in modes.keys()]))

if isinstance(data, str):
    result = modes[mode](data)
else:
    data = list(data)
    result = [modes[mode](entry) for entry in data]
    return result

answered Nov 3, 2021 at 16:42

Lecker Spam

314 bronze badges

2 Comments

Matiiss Over a year ago

you should unindent the return, otherwise it will return result only if data is not a string, and result = None is not necessary, you will always get something as a result, alternatively you can do sth like return result or None if you want to explicitly return None in case for example result is an empty list

Lecker Spam Over a year ago

yeah right, thanks, i think this was a copy and paste thing ^^

Collectives™ on Stack Overflow

Refactoring Python Code when the only thing changes are methods?

5 Answers 5

Comments

Comments

2 Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

2 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related