ruby-style-guide icon indicating copy to clipboard operation
ruby-style-guide copied to clipboard

Unspecified Style point: obj.is_a? vs obj.class == Class

Open devonparsons opened this issue 10 years ago • 8 comments

Although the case equality operator === is mentioned, of the two methods of class checking demonstrated here:

"this string".is_a? String  # => true
"this string".is_a? Object  # => true
4.is_a? Object              # => true

"this string".class == CSV  # => false
4.class == Fixnum           # => true
4.class == String           # => false

Neither are stated as preferred. I think it's fairly well agreed that is_a? is the the more Ruby way of doing things, but I've seen obj.class == Classname fairly often as well and it's easily readable.

devonparsons avatar Feb 18 '15 15:02 devonparsons

Narrowing the type check feels like an anti-pattern to me, as it reduces the flexibility of the code. I'd advise against the use of explicit class checks.

Let's see what the others will have to say about this.

bbatsov avatar Feb 18 '15 17:02 bbatsov

I'd say it has nothing to do with style, because .is_a? klass and .class == klass have just different meaning and should be used according to situation/needs.

Example 1:

4.is_a? Numeric         # => true
Math::PI.is_a? Numeric  # => true

Example 2:

4.class == Fixnum         # => true
Math::PI.class == Fixnum  # => false

Both examples reflect legitimate needs of real world problems and both should be used where needed. Can't see any possible guided preference here.

DanielVartanov avatar Feb 18 '15 21:02 DanielVartanov

Both examples reflect legitimate needs of real world problems and both should be used where needed. Can't see any possible guided preference here.

I don't see why in the second case you wouldn't want to use is_a? as well. While there may be some super rare cases when an explicit class check is truly needed this example doesn't seem like one of them.

bbatsov avatar Feb 19 '15 06:02 bbatsov

Wrong example, my bad, I was writing it at night. I meant that is_a? checks whether an instance belongs to a given class or its ancestors, sometimes it is needed, sometimes not.

DanielVartanov avatar Feb 19 '15 08:02 DanielVartanov

@DanielVartanov Right, I agree that the functionality is not the same because .class == Classname doesn't check inheritance. I specifically meant the case where you are checking the absolute class of the object, where either method would have identical results. My original post was unclear about this.

By the way, if anyone is interested in a concrete example, I wrote this on a Stack Overflow answer:

csv << ['a', 123, 1.5, '0123'].map{ |e| e.class == String ? "\"#{e}\"" : e } # Ensure strings have quotes around them

And someone pointed out that it would be more appropriate to use is_a?. This prompted me to look at the style guide and then open this issue.

devonparsons avatar Feb 19 '15 14:02 devonparsons

@devonparsons, yes I see what you mean after reading your answer at stackoverflow. I do agree that is_a? was more appropriate in that case. People do derive from Hash, Array, or even String sometimes, so it is more safe to take inheritance into account.

Usage of .class == is probably to be encouraged only when one knows what they are doing. For other (probably most) cases is_a? is the first option to pick.

DanielVartanov avatar Feb 19 '15 15:02 DanielVartanov

I agree that inheriting from base classes is a common enough occurance that the point should be made explicit to promote safety.

So perhaps an appropriate style point should read something like:

Prefer Object#is_a? over direct class checking to allow for inheritence:

class CustomString < String
end
a = CustomString.new("text")

# bad 
a.upcase! if a.class == String # => text

# good
a.upcase! if a.is_a? String    # => TEXT

I'm sure @bbatsov can write that better than me but we get the gist

devonparsons avatar Feb 19 '15 15:02 devonparsons

Yeah, we can definitely add something along those lines, although I wouldn't use an example involving inheritance from String, as doing so is rarely a good idea. :-)

A related rule might be that for API it's better check for capabilities than for types - e.g. if you check that something responds_to << your API will potentially work with both strings and arrays.

bbatsov avatar Feb 19 '15 15:02 bbatsov