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: invert some conditionals for better readability #335

Merged
merged 1 commit into from Jun 1, 2022

Conversation

agilgur5
Copy link
Collaborator

@agilgur5 agilgur5 commented May 31, 2022

Summary

Small refactor to invert some conditionals/if statements for better readability

  • handful of refactors I'm making as I'm now very familiar with the codebase to make it easier to work with and more approachable to new contributors as well

Details

  • a few if (cond) { big block } return could be inverted to if (!cond) return then the block un-indented instead

    • generally speaking, this makes it a lot more readable (less indentation, etc) and follows the existing code style in much of the codebase, where it's a few quick one-line ifs and then just the rest of the code
  • shorten the resolvedFileName conditional by using optional chaining

    • prefer the simpler x?.y over the older x && x.y syntax
  • add a resolved variable for less repetition of the whole statement

  • add a comment to the pathNormalize line about why it's used in that one place and link to the longer description in the PR as well

  • shorten comment about useTsconfigDeclarationDir so it doesn't take up so much space or look so important as a result

    • it's easier to read this way and I made the explanation less verbose and quicker to read too
    • already have comments and lines in the codebase longer than 80 chars, so that's not a guideline that's followed anywhere else
  • remove the else there and just add an early return instead, similar to the inverted conditionals above

    • similarly makes it less unindented, more readable etc

Review Notes

- a few `if (cond) { big block } return` could be inverted to
  `if (!cond) return` then the block unindented instead
  - generally speaking, this makes it a lot more readable (less
    indentation, etc) and follows the existing code style in much of the
    codebase, where it's a few quick one-line ifs and then just the rest
    of the code

- shorten the resolvedFileName conditional by using optional chaining
  - prefer the simpler `x?.y` over the older `x && x.y` syntax
- add a `resolved` variable for less repetition of the whole statement
- add a comment to the `pathNormalize` line about why it's used in that
  one place and link to the longer description in the PR as well

- shorten comment about `useTsconfigDeclarationDir` so it doesn't take
  up so much space or look so important as a result
  - and it's easier to read this way and I made the explanation less
    verbose and quicker to read too
- remove the `else` there and just add an early return instead, similar
  to the inverted conditionals above
  - similarly makes it less unindented, more readable etc
@agilgur5 agilgur5 added the kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs label May 31, 2022
@ezolenko ezolenko merged commit 01272d3 into ezolenko:master Jun 1, 2022
@agilgur5
Copy link
Collaborator Author

agilgur5 commented Jun 9, 2022

I was looking through some old tabs and saw that CodeClimate actually defines a term for this kind of complexity. "Cognitive Complexity" includes complexity due to nesting.

Great to have a term for what I was describing moving forward!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants