Fix DateToDay #1126
Fix DateToDay #1126
Conversation
Change the algorithm to Zeller’s Congruence refer link - (https://www.geeksforgeeks.org/zellers-congruence-find-day-date/)
The test case '18/02/2001' and '01/01/2001' were Invalid. I have corrected them and also attached the calendar link for reference (https://www.timeanddate.com/calendar/?year=2001&country=35)
First time contribution, sorry for the inconvenience.
Made year--
Conversions/DateToDay.js
Outdated
| 11: 9, | ||
| 12: 10 | ||
| } | ||
|
|
||
| // show the week day in a number : Sunday - Saturday => 0 - 6 | ||
| const daysNameList = { // weeks-day |
There was a problem hiding this comment.
Please use an array here.
Conversions/DateToDay.js
Outdated
| // check the data are valid or not. | ||
| if (day < 0 || day > 31 || month > 12 || month < 0) { | ||
| return new TypeError('Date is not valid.') | ||
| } | ||
| // divide year to century and yearDigit value. | ||
| const yearDigit = (year % 100) | ||
| // date is resolved based on Zeller's congruence. |
There was a problem hiding this comment.
That's the overarching comment. This should be specific to the following line: "Count Jan & Feb as months 13 & 14 of the previous year".
There was a problem hiding this comment.
Ohhh great input, I will change it.
Updated the comment and renamed the daysNameList to daysNameDict. If there is any changes to be done please let me know, if not then please accept my PR and merge this. I hope I did a great job as this is my first time exposure to open source.
Conversions/DateToDay.js
Outdated
| // show the week day in a number : Sunday - Saturday => 0 - 6 | ||
| const daysNameList = { // weeks-day | ||
| const daysNameDict = { // weeks-day |
There was a problem hiding this comment.
Make this an array please: Keys are 0 to 6.
There was a problem hiding this comment.
Changed it to an array.
Changed List to Array and also fixed to trailing spaces.
|
Hi, can you please have a look at my last commit and if everything thing is correct, can you approve my PR I need these points for Hacktober 2022 :P, thanks in advance. |
Conversions/DateToDay.js
Outdated
| // return the weekDay name. | ||
| return daysNameList[weekDay] | ||
| year %= 100 | ||
| const weekDay = (year + Math.floor(year / 4) + Math.floor(century / 4) - 2 * century + Math.floor((26 * (month + 1)) / 10) + day - 1) % 7 |
There was a problem hiding this comment.
You don't need the % 7 here if you apply % 7 below.
There was a problem hiding this comment.
Hmmm understood, changed it.
I am torn on whether I should merge this or #1125. Initially this seemed like the better option, but the other PR has improved significantly in response to my review comments and is mostly on par with this one. |
removed %7 from line 36
Do mine :P, just kidding. I guess, merge the one that is better in terms of performance. Honestly I got to learn a lot from just this one contribution. If there is any other (good first issue) then maybe assign it to me :). |
|
If you decide to merge the other PR can you label mine as hacktoberfest-accepted that will give me at least 1 point 😀 |
|
Closing as #1125 has been merged. Sorry. Thank you for your effort. |
No problem, I learnt a lot thankyou for all the inputs 😀 |
Fix implementation of Zeller's congruence
refer to https://www.geeksforgeeks.org/zellers-congruence-find-day-date/
Describe your change:
Checklist:
Example:
UserProfile.jsis allowed butuserprofile.js,Userprofile.js,user-Profile.js,userProfile.jsare notFixes: #{$ISSUE_NO}.