TheOdinProject / javascript-exercises

MIT License
1.28k stars 34.52k forks source link

README.md hint doesn't match solution.js #482

Closed jveneziano25 closed 4 months ago

jveneziano25 commented 4 months ago

In Exercise 06 - leapYears, the README file has a hint that says "use an if statement and && to make sure all the conditions are met properly". However, the solution.js file doesn't even use an if statement, which can lead to confusion.

MaoShizhong commented 4 months ago

Thanks @jveneziano25. Agreed that the current solution will be better replaced with something that's more intuitive and recognisable for most people at this point in the curriculum, i.e. using conditionals rather than returning a single complex boolean expression.

The solution I'd propose is:

const leapYears = function (year) {
  const isYearDivisibleByFour = year % 4 === 0;
  const isCentury = year % 100 === 0;
  const isYearDivisibleByFourHundred = year % 400 === 0;

  if (
    isYearDivisibleByFour &&
    (!isCentury || isYearDivisibleByFourHundred)
  ) {
    return true;
  } else {
    return false;
  }
};

module.exports = leapYears;

Let me know if you're happy to open a pull request with this change to the exercise's solution file or not, then I can assign you or open this up for assignment.

jveneziano25 commented 4 months ago

Sure, I would be happy to open a pull request for this and I think your suggestion is much clearer for the current structure of the program.

Get Outlook for iOShttps://aka.ms/o0ukef


From: MaoShizhong @.> Sent: Wednesday, June 26, 2024 3:30:30 PM To: TheOdinProject/javascript-exercises @.> Cc: jveneziano25 @.>; Mention @.> Subject: Re: [TheOdinProject/javascript-exercises] README.md hint doesn't match solution.js (Issue #482)

Thanks @jveneziano25https://github.com/jveneziano25. Agreed that the current solution will be better replaced with something that's more intuitive and recognisable for most people at this point in the curriculum, i.e. using conditionals rather than returning a single complex boolean expression.

The solution I'd propose is:

const leapYears = function (year) { const isYearDivisibleByFour = year % 4 === 0; const isCentury = year % 100 === 0; const isYearDivisibleByFourHundred = year % 400 === 0;

if ( isYearDivisibleByFour && (!isCentury || isYearDivisibleByFourHundred) ) { return true; } else { return false; } };

module.exports = leapYears;

Let me know if you're happy to open a pull request with this change to the exercise's solution file or not, then I can assign you or open this up for assignment.

— Reply to this email directly, view it on GitHubhttps://github.com/TheOdinProject/javascript-exercises/issues/482#issuecomment-2192572058, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AR47NZYIDG5NX73ES4SMSE3ZJMQGNAVCNFSM6AAAAABJ6QTWAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSGU3TEMBVHA. You are receiving this because you were mentioned.Message ID: @.***>

MaoShizhong commented 4 months ago

Go for it