403e57e2be130d2218f992b86dfa8260

I'm pretty sure we can figure out a shorter and more elegant way to do this :-D

1
2
3
4
5
6
Object.extend(Date.prototype, {
  isLeap: function(){
    var year = this.getFullYear();
    return (year % 4 == 0 && year % 100 != 0) || (year % 100 == 0 && year % 400 == 0);
  }
});

Refactorings

No refactoring yet !

Bfec5f7d1a4aaafc5a2451be8c42d26a

macournoyer

September 21, 2007, September 21, 2007 07:30, permalink

No rating. Login to rate!

I'm pretty sure the last year % 100 == 0 is not needed as we already test w/ a multiple of 100 (400), plus we now can remove parenthesis.

1
2
3
4
5
6
Object.extend(Date.prototype, {
  isLeap: function(){
    var year = this.getFullYear();
    return year % 4 == 0 && year % 100 != 0 || year % 400 == 0;
  }
});
403e57e2be130d2218f992b86dfa8260

Gary Haran

September 21, 2007, September 21, 2007 08:05, permalink

2 ratings. Login to rate!

Indeed we could remove one of the conditions. I replaced the condition with an integer value test instead, because JS automatically assumes that any value other than 0 is true. Unfortunately it means more parenthesis but it still makes the code shorter.

Can we find any other ways of making this shorter? :-D

PS: you should add a way to add more than one code snippet so you can show two pieces of code that work together. I'd love to show the test portion of this code in the same thread.

1
2
3
4
5
6
Object.extend(Date.prototype, {
  isLeap: function(){
    var year = this.getFullYear();
    return !(year % 4) && !!(year % 100) || !(year % 400);
  }
});
Bfec5f7d1a4aaafc5a2451be8c42d26a

macournoyer

September 21, 2007, September 21, 2007 08:20, permalink

No rating. Login to rate!

I'll try to add sections with ## like pastie, thx for the feedback Gary.
But that sure is a lot of !!(!negation != !false)

403e57e2be130d2218f992b86dfa8260

Gary Haran

September 21, 2007, September 21, 2007 21:23, permalink

1 rating. Login to rate!

The reason behind all those is purely to return a true boolean at the other end because that's what would be expected. One of the aims of any library code I write is to have as short a footprint as possible. Because I unit test functions like these I don't mind losing a bit of readability. Btw in the prototype source code uses the !! trick a few times to convert undefined to a boolean value.

Bfec5f7d1a4aaafc5a2451be8c42d26a

macournoyer

September 23, 2007, September 23, 2007 13:13, permalink

1 rating. Login to rate!

Nice tip Gary! I didn't know about this idom!

And about sections, how about that:

Ruby code

1
2
3
def code
  puts 'cool'
end

View (rhtml)

1
<h1><%= @title %></h1>

Javascript

1
2
3
4
5
var Page = {
  omg: function() {
    alert('img');
  }
};
Avatar

br

September 28, 2007, September 28, 2007 00:45, permalink

6 ratings. Login to rate!
1
function isLeap(y){return new Date(y, 2, 0).getDate()==29;}
90dfcf7e54299842383ab503b6df1a65

Ali Karbassi

October 2, 2007, October 02, 2007 19:03, permalink

No rating. Login to rate!

Going of what "br" said and other people's code, we could do this the following.

1
2
3
4
5
6
Object.extend(Date.prototype, {
	isLeap: function()
	{
		return new Date(this.getFullYear(), 2, 0).getDate() == 29;
	}
});
D14b68d8b26a558f5acea3ceccf9c78c

Owen

October 7, 2007, October 07, 2007 19:07, permalink

1 rating. Login to rate!

Since all leap years are multiples of 4, you don't need anything but %4. Right? Seems overly complicated to me.

1
2
3
4
5
Object.extend(Date.prototype, {
  isLeap: function(){
    return !(this.getFullYear() % 4);
  }
});
90dfcf7e54299842383ab503b6df1a65

Ali Karbassi

October 13, 2007, October 13, 2007 19:08, permalink

No rating. Login to rate!

Owen:

According the Wikipedia, it's not JUST every 4 years.

if year modulo 400 is 0 then leap
else if year modulo 100 is 0 then no_leap
else if year modulo 4 is 0 then leap
else no_leap

1f292232d7d725c0111b15193e38ff87

sameer

October 24, 2007, October 24, 2007 05:14, permalink

No rating. Login to rate!

wikkipedia code has some flaw in it.the years 100,200,300 are leap years,but according to the code output these are not leap years.an anyone explain that?

Your refactoring





Format Copy from initial code

or Cancel