Skip to content

Improve code quality with static analysis#119

Merged
quaquel merged 6 commits intoquaquel:masterfrom
EwoutH:static-analysis
May 3, 2022
Merged

Improve code quality with static analysis#119
quaquel merged 6 commits intoquaquel:masterfrom
EwoutH:static-analysis

Conversation

@EwoutH
Copy link
Copy Markdown
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
Copy Markdown

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.

3 participants