Reducing code smelling with a help from first class citizens
Recently I've encountered a situation where elif's stacked on top of each other like pancakes. For example, we are parsing different file formats. Our parsers are ready and we just need a higher level interface.
if file_format == 'xml':
parse_xml(file_name)
elif file_format == 'csv':
parse_csv(file_name)
elif file_format == 'json':
parse_json(file_name)
else:
parse_txt(file_name)
At first there were just if and else. Then things started to add up and I realized that multiple elif's was just a smelling piece of code.
To deal with that we can leverage a python concept that functions are first class citizens. Meaning we can return them or use them as a dictionary values. So we need to map our file formats with corresponding parsing functions:
parsers = {
'xml': parse_xml,
'csv': parse_csv,
'json': parse_json,
}
Notice that I did not include our default txt parser. It's like we rewrote
if and elif's of our previous code. Now we need a function to return a parser for a given file format:
def get_parser(file_format):
return parsers.get(file_format, parse_txt)
This will return a parser function from our parsers dictionary for a given file format. Or, if file_format is not a key of that dictionary, default parse_txt. As an analogy before, it's like we rewrote the last else statement.
To use that we need to call get_parser function and we can also immediately execute it:
get_parser('csv')(file_name)
But if you want to be more transparent, you can expand this to multiple lines:
parser = get_parser('csv')
parser(file_name)
I think this code looks better than at the beginning of the article, although might not be obvious for everyone, so don't forget to place some comments :)
And I want to believe that it works a little faster because we're just getting a value from a dictionary instead of checking every single if case.
Top comments (5)
I still don't really understand why
eliforelse ifsmells, when dictionary with functions just make the code more complicated.Danger 1 - Dictionary with non-function gets compiled
Danger 2 - not in Python. Fall through in switch case statement
Because the number of code paths is reduced, and the intent of the code is way more clearer (get me a parser for this format). The code is actually simpler, because less chance for the unexpected.
Complexity is not just what is, but what could be.
I have nothing against
elifs, it's a powerful and useful tool. And I use it a lot in my projects. But when I see 4-5 or more of them - Idk, it makes me cringe. Dictionary can help with that giving you a neat structure ofwhatto do when you havethiscondition.I absolutely agree. In wemake-python-styleguide we have a special violation called
TooManyElifViolations. We raise a violation when developers use more then 3elifs.More docs: wemake-python-stylegui.de/en/lates...
Check it out:
The strictest and most opinionated python linter ever!
wemake-python-styleguide
Welcome to the strictest and most opinionated python linter ever.
wemake-python-styleguideis actually a flake8 plugin with some other plugins as dependencies.Quickstart
You will also need to create a
setup.cfgfile with the configuration.We highly recommend to also use:
Running
This app is still just good old
flake8And it won't change your existing workflow.See "Usage" section in the docs for examples and integrations.
We also support Github Actions as first class-citizens Try it out!
What we are about
The ultimate goal of this project is to make all people write exactly the same
pythoncode.I agree with you. But this code was just an example. I could not come up with something more useful but had an idea to write about.