Add "instance_of? vs class comparison" section

[fatkodima]

rubocop/rubocop#8833 (comment)

[bbatsov]

Thanks!

[bquorning]

Neither this PR nor rubocop/rubocop#8833 mentions why using Object#instance_of? is better than comparing classes. In particular, I am curious why var.class == Date is considered bad.

[fatkodima]

var.class == Date is not that bad as var.class.name == "Date", because in the latter it is very easy to make an unnoticed typo. So it is better to use a specifically made method for both of them, for consistency, safety and expresiveness.

[marcandre]

I can't say I agree with this rule, mostly because all of these forms are bad. One should typically use is_a?, not instance_of?. There's rarely a compelling reason to exclude subclasses.

If it is one of those rare cases where it strictly needs to be a particular class, than I might prefer foo.class == Bar. It has the advantages of being shorter, of stressing the difference and not relying on the assumption that the reader knows what instance_of? does exactly (lower cognitive load).

I'd second a rule that says to never use instance_of?.

[bquorning]

I just ran into this issue again, now this code: value.class == TrueClass. We are checking if the response from an API call is of the right type so we don’t insert e.g. a "true" string into the database. In this case, value.instance_of?(TrueClass) and value.is_a?(TrueClass) didn’t sit right with me, so I went for value.equal?(true) instead. I wonder if the booleans warrant a special mention in the style guide (and the cop)?

[marcandre]

@bquorning I wouldn't mention it, as I think most people would not think about doing a check on class but simply value == true (or eql? / equal?).

[swrobel]

var.class == Date is not that bad as var.class.name == "Date", because in the latter it is very easy to make an unnoticed typo. So it is better to use a specifically made method for both of them, for consistency, safety and expresiveness.

I actually find var.class.name == "Date" useful in situations where autoload order can be an issue, typically in Rails apps. I don't necessarily want to autoload Date at the point where this comparison is defined, and this allows me to avoid doing so by referencing the string representation of the class name, rather than the class itself.