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

refactor #1

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

refactor #1

wants to merge 1 commit into from

Conversation

H2148
Copy link

@H2148 H2148 commented Dec 20, 2021

Overall really good, you've got all the right stuff going on! I like the debug stuff too, use of dictionaries also neat 👍

Here's a few things I would change; it is of course personal though, and there are probably 10 different ways to do this

Some of my reasoning behind the changes

  • use more explicit names for stuff (for your own sanity when you look at it after some time has passed, or even just to save brain power)
  • refactor code out into separate functions, especially when there are multiple things going on
  • try to get functions doing one thing only; it's tricky though, what that one thing is, can be hard to define. I generally think it is a "code smell" if a function is more than 10 lines (depends a bit on the language) and/or multiple loops (especially nested ones) and/or code that is like "if this, then that, but it could be this in which case do the other thing, and then finally"
  • no need for comments, it can help force yourself into write the code in an understandable way, i.e, "if skip_download(filepath): continue", "for category in categories".

Code might not actually be working too, I didn't run it : p

The essence is all there, there's a few things I would change though; it is of course personal though
Some of my reasoning behind the changes
- try to use more explicit names for stuff
- refactor code out into separate functions, especially when there are multiple things going on
- try to get functions doing one thing only; it's tricky though, sometimes that one thing is hard to define. I generally notice if a function is more than 10 lines and/or multiple loops (especially nested ones) and/or code that is like "if this, then that, but it could be this in which case do the other thing, and then finally"
- no need for comments, it can help force yourself into write the code in an understandable way, i.e, "if skip_download(filepath): continue", "for category in categories".

Code might not actually be working too, I didn't run it : p
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.

1 participant