Hi everyone!
This article will be dedicated to refactoring and clean code practices.
Let's start with the task I kindly took from the awesome project for developers and the possibility to mentor, learn different practices and languages and polish your skills instantly - Exercism.org.
So, here is the task related to clarifying whether the provided year is a leap or not:
Given a year, report if it is a leap year.
The tricky thing here is that a leap year in the Gregorian calendar occurs:
on every year that is evenly divisible by 4
except every year that is evenly divisible by 100
unless the year is also evenly divisible by 400For example, 1997 is not a leap year, but 1996 is. 1900 is not a leap year, but 2000 is.
My first solution was the next:
function isLeap(year: number) {
return year % 400 === 0 ? true : year % 100 === 0 ? false : year % 4 === 0 ? true : false
}
Pros:
One liner?
Not actually, but quite concise I would say. The true one-liner can look like this (Close your eyes!):
const isLeap = (y: number) => !(y % 400) || (!!(y % 100) && !(y % 4));
Cons:
Hard to understand.
As you can probably agree - it's long and barely readable and easily understandable.
Redundant code in the end (did it mainly to the love to ternary operators)
Possible improvements:
We can provide a bit of formatting and grouping, though it will not be a liner anymore. And it's ok. Also, it can be helpful for future refactoring when more logic or improvements need to be done
const isLeap = (year: number) =>
year % 400 === 0 ? true :
year % 100 === 0 ? false :
year % 4 === 0;
Not that it's still concise but more readable and logically split (pov). If evenly divisible by 400 then it's a leap. If by 100 - not and so on.
The Second solution (Clean Code):
After the Clean Code (the site is a bit outdated, but the practices are still valuable as never before) approach applying it would look like this:
const isLeap = (year: number): boolean => {
const isDividedBy = (divider: number): boolean => year % divider === 0;
return isDividedBy(400) || (!isDividedBy(100) && isDividedBy(4));
}
As you can see - there is not so much more code added, but readability is much higher. Now I can simply read: "Year has leapt if it's divided evenly by 400 OR NOT divided by 100 WHILE divided by 4".
Hope it was a bit of fun for you!
Top comments (1)