-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
refactor: reduce code duplication in FloodFill
#1645
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: reduce code duplication in FloodFill
#1645
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also consider making checkLocation
return a boolean - it could return whether the location is valid.
Then you'd be duplicating the two throw
s (but this is actually not that bad, because such side effects being "hidden" in a function may be a bit surprising).
You would then be able to deduplicate the x >= 0 && x < rgbData.length && y >= 0 && y < rgbData[0].length
bounds check as well.
Co-authored-by: appgurueu <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1645 +/- ##
==========================================
+ Coverage 84.07% 84.08% +0.01%
==========================================
Files 375 375
Lines 19688 19685 -3
Branches 2913 2917 +4
==========================================
+ Hits 16552 16553 +1
+ Misses 3136 3132 -4 ☔ View full report in Codecov by Sentry. |
f405dde
to
1be1853
Compare
@appgurueu thanks for 1be1853. |
checkLocation
to FloodFill
FloodFill
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I forgot to approve, didn't I...
Describe your change:
This PR reduced the code duplication and adds missing tests.
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.