-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
solution: Project Euler Problem 19 #1174
base: master
Are you sure you want to change the base?
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.
Looks good, but please add some comments.
Added the comments and refactored to make more legible |
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.
Turning the terse leap year condition into an if
-else
construction that takes about twenty times as much vertical space (!) is pretty bad. If anything, the current construction should be made to use a ternary rather than relying on coercion from bool to 0 or 1.
The rename of n
to numberOfSundays
does indeed improve readability though.
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.
Changed to a ternary if
offtopic: @appgurueu i saw that you like Lua, a professor at my university studied in the department that developed the Lua language at PUC-Rio 😄 |
Oh that's nice! |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Describe your change:
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.