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

Freeze Refactor - Diminish Cyclomatic Complexity #583

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

calebe-bertoluci
Copy link

@calebe-bertoluci calebe-bertoluci commented Nov 19, 2024

Hello!
I would like to contribute to TinyDB by reducing the cyclomatic complexity of one of its methods.

I used the Codalyze Tool to statically analyze the code, and after extracting the result from all the software, it turns out that the freeze method has the highest cyclomatic complexity in TinyDB.
After the refactoring, it was possible to obtain a score of 6, for 3 points, on a scale of 0 to 10.

I hope you'll consider this change, and I'm excited to contribute to TinyDB.

Codalyze Tool: https://github.com/selcukusta/codalyze-rest-api
Method Tests Passing 100% ✅

@msiemens
Copy link
Owner

msiemens commented Dec 3, 2024

Hey @calebe-bertoluci, thanks for your PR! To be honest, to me the original looks cleaner even though it might have a higher cyclomatic complexity score. It is a simple if-else and not a Python dict that is used as a dispatch table (which is then immediately thrown away). Sure, the disptach table you're proposing has less conditionals, but I'd argue it's harder to understand it compared to the original (it requires recognizing the code pattern in the first place, whereas the if-else is very easy to understand). What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants