Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
devonparsons opened this issue Feb 18, 2015 · 8 comments
Open

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

devonparsons opened this issue Feb 18, 2015 · 8 comments

Comments

@devonparsons
Copy link

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.

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 18, 2015

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.

@DanielVartanov
Copy link

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.

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 19, 2015

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.

@DanielVartanov
Copy link

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.

@devonparsons
Copy link
Author

@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.

@DanielVartanov
Copy link

@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.

@devonparsons
Copy link
Author

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

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 19, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants