6
\$\begingroup\$

I did a line-by-line translation and am wondering if the JavaScript code looks idiomatic.

Original Ruby code:

class Year
  def initialize(number)
    @number = number
  end

  def leap?
    divisible_by?(400) || ( divisible_by?(4) && !divisible_by?(100))
  end

  private
  def divisible_by?(a_number)
    @number % a_number == 0
  end  
end 

JavaScript Implementation looks like this:

var Year = function( number ) {      
  this.number = number;    
  function isLeap() {
    return isDivisibleBy(400) || ( isDivisibleBy(4) && !isDivisibleBy(100));
  }
  function isDivisibleBy(a_number) {
    return this.number % a_number == 0
  }        
  return isLeap();
}
\$\endgroup\$

2 Answers 2

7
\$\begingroup\$

Unfortunately, the two pieces of code are not equivalent, so definitely not idiomatic. Javascript does not have classes like other languages, so it has a unique way of creating class equivalents.

Let's look at what your Javascript code does first, so we can understand how to fix it.

var Year = function( number ) {

Going ok so far. We could also simply declare Year as a function: function Year(number) {..., but this is acceptable.

  this.number = number;

Also good. When we say new Year(2014), we'll assign it to the number property of the instance.

  function isLeap() {
    return isDivisibleBy(400) || ( isDivisibleBy(4) && !isDivisibleBy(100));
  }

Ok, so the method body is fine, but this is not being assigned as a method on the object, so we won't be able to do something like yearInstance.isLeap(). Instead, isLeap can only be used from the context of this method.

  function isDivisibleBy(a_number) {
    return this.number % a_number == 0
  }

This is meant to be a private method. Unfortunately, Javascript does not have a traditional concept of private scope. More on this later.

  return isLeap();
}

This return statement is...tricky. In Javascript, we can return an object from a constructor, and the value returned would be that object and not an instance of the function we are instantiating. I'll admit, that's confusing. Here's an example:

function A() {
    return new B();
}

Now, if we call new A(), we actually get back a new B. But there are caveats. If the return value is not an object (so a primitive like a boolean or a number not boxed), then we will not get back the return value, we'll get the object we are trying to instantiate. So now back to your code. From the constructor, we return the result of a call to isLeap. First, isLeap is not being called on any object, so it will always return false. Because false is a primitive, the result is discarded, and we end up exactly with what we expect. Because of the trickiness of Javascript constructors and returns, even if we get what we want, we should still not have a return statement. It is inappropriate in a constructor, and can lead to unexpected bugs. For more, see this StackOverflow answer.


Now we can break this code down to be more idiomatic.

function Year(number) {
    this.number = number;
}

Year.prototype.isLeap = function() {
    return this.isDivisibleBy(400) || (this.isDivisibleBy(4) && !this.isDivisibleBy(100));
}

Year.prototype.isDivisibleBy = function(aNumber) {
    return this.number % aNumber === 0;
}

Here, we've broken out the methods from the constructor. Because Javascript is a prototypical language, we create methods by adding them to the prototype of the constructor, Year.prototype in this case. We see isLeap is now a property on the Year prototype, meaning we can now call it like yearInstance.isLeap(). isDivisibleBy is also a property on the Year prototype. This means that, here, it's a public method as well. Like I said before, Javascript does not have the same concept of private visibility.

We can't make a private method as a property on the object. There are a few ways around this. The simplest is to prepend the method name with an underscore (like Year.prototype._isDivisibleBy). This does not make the method uncallable, but is a common convention to indicate that the method is meant to be private.

An alternate way (which I am not going to show) is to use a closure to hide the function.

Inside isLeap, all of the calls to isDivisibleBy must now be prefixed with this. because it is now a method.

Inside isDivisibleBy, we now see == is replaced with ===. This is not required in any way, and can sometimes boil down to personal preference (see JSLint). Basically, it compares value and type when checking for equality. Also, a_number has been changed to aNumber. Generally, I find that Javascript following similar camelCasing conventions as Java.

\$\endgroup\$
3
\$\begingroup\$

Since cbojar didn't want to show a solution with closures I will do.

function Year(number) {
 // Check number here

 function isDivisibleBy(aNumber) {
  return number % aNumber === 0;
 };

 return {
  get year() { return number; },
  get isLeap() {
    return isDivisibleBy(400) || (isDivisibleBy(4) && !isDivisibleBy(100));
  }
 }
};

You can access isLeap as property.

var currentYear = new Year(new Date().getFullYear());
> currentYear.isLeap;
false
> currentYear.year;
2014

The use of closures hides the isDivisibleBy function and prevents anybody from changing the year.

> // isDivisibleBy is not accessible from outside
> currentYear.isDivisibleBy(0);
TypeError: undefined is not a function
> // Cannot change year
> currentYear.year = 2000;
2000
> currentYear.year;
2014
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks! I didn't want my answer to get too long and complicated, so I really appreciate you showing this separately. :) \$\endgroup\$
    – cbojar
    Oct 9, 2014 at 16:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.