Refactoring. The beginning.

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 400

For 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!

Comments

[Jerry]