Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve code quality with static analysis #119

Merged
merged 6 commits into from May 3, 2022

Conversation

EwoutH
Copy link
Collaborator

@EwoutH EwoutH commented May 3, 2022

I ran some static analysis over the codebase (using https://deepsource.io/) to improve the code quality. It mainly removes some unnecessary complex constructs, like lambda expressions, list/dict comprehension and super class methods, where they aren't necessary. This improves performance and readability.

Each commit has a description what is changed and why, if you got the time I would recommend reading through those (I learned quite a few new things)!

If there are things you do not like or prefer, please let me know, and I will remove them from this PR.

A lambda that calls a function without modifying any of its parameters is unnecessary. Python functions are first-class objects and can be passed around in the same way as the resulting lambda. It is recommended to remove the lambda and use the function directly.

Example:
hashify = lambda x: hash(x)

It is preferred to use the function hash directly as the lambda function is calling the same function without any modifications. Another reason to avoid lambda function here is that it makes the debugging difficult. Lambdas show as <lambda> in tracebacks, where functions will display the function’s name.
It is unnecessary to use a comprehension just to loop over the iterable and create a list/set/dict out of it. Python has a specialized set of tools for this task: the list/set/dict constructors, which are faster and more readable.
Any method in a child class which is only calling the overridden method of any of its base class using super and doing nothing else is unnecessary. If the method isn't present in the child class, Python implicitly looks for the method in its base classes.

This issue is valid only if the method in the child class and the method in the base class has the same signature.
To check if a variable is equal to one of many values, combine the values into a tuple and check if the variable is contained in it instead of checking for equality against each of the values. This is faster, less verbose, and more readable.
Iterate the dictionary directly instead of calling .keys(). Using for key in dictionary would always iterate the dictionary keys.
Instead of adding items to a dictionary just after creation, it should be refactored to add those items into the dict definition itself.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 80.466% when pulling b4ce855 on EwoutH:static-analysis into ec838cd on quaquel:master.

@quaquel quaquel merged commit 6f4a4ae into quaquel:master May 3, 2022
@EwoutH EwoutH mentioned this pull request May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants