Skip to content

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!

@agilgur5 agilgur5 deleted the refactor-invert-ifs branch July 2, 2023 21:26
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.

2 participants