refactor: invert some conditionals for better readability #335
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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
added
the
kind: internal
This was referenced Jun 1, 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Small refactor to invert some conditionals/
ifstatements for better readabilityDetails
a few
if (cond) { big block } returncould be inverted toif (!cond) returnthen the block un-indented insteadshorten the
resolvedFileNameconditional by using optional chainingx?.yover the olderx && x.ysyntaxadd a
resolvedvariable for less repetition of the whole statementadd a comment to the
pathNormalizeline about why it's used in that one place and link to the longer description in the PR as wellshorten comment about
useTsconfigDeclarationDirso it doesn't take up so much space or look so important as a resultremove the
elsethere and just add an early return instead, similar to the inverted conditionals aboveReview Notes
This is unfortunately going to merge conflict with fix: don't attempt to change declarationMap
sourceswhen no output #334 as one of the conditionals is changed there as well, so I'll need to rebase this once that's fixedIgnoring whitespace changes makes this PR easier to read