19

I see the issue with using class variables with Ruby; however, it seems RuboCop's documentation for how to fix the issue is not sufficient.

Now, I could just ignore it. Given my project, it doesn't matter. But, I just want to know what Rubocop is trying to tell me to do, because it doesn't make sense.

Executing the provided code in irb 0.9.6 with Ruby 2.5.1 gives:

class A
  @test = 10
end
#=> 10
class A
  def test
    @@test # you can access class variable without offense
  end
end
#=> :test
A.new.test
Traceback (most recent call last):
        3: from /Users/Ricky/.rbenv/versions/2.5.1/bin/irb:11:in `<main>'
        2: from (irb):12
        1: from (irb):9:in `test'
NameError (uninitialized class variable @@test in A)
Did you mean?  @test

So, no. We obviously cannot access class variable without offense. irb was very offended. But, ruby suggests using @test. Maybe it was just a typo? Let's try it:

class A
  @test = 10
  def test
    @test # you can access class variable without offense
  end
end
#=> :test
A.new.test
#=> nil

So, the instance variable was never defined. What is RuboCop trying to say here?

RWDJ
  • 734
  • 8
  • 13

3 Answers3

17

You are missing the difference between the scopes of variables.

class A
  @test = 42
end

The above declares an instance variable in the class scope. It is accessible as

A.instance_variable_get(:@test)
#⇒ 42

You can define an accessor for this variable:

class A
  @test = 42
  def self.test
    @test
  end
end

A.test #⇒ 42

It is shared between instances and to access it from instances you should refer to the class:

#     ⇓⇓⇓⇓⇓ HERE
A.new.class.test #⇒ 42

The following code declares an instance variable on the class instances:

class A
  def initialize
    @test = 42
  end
end

It can be accessed from the instances of A:

A.new.instance_variable_get(:@test)
#⇒ 42

Class variables have some drawbacks when used within the class hierarchy, that is [probably] why Rubocop suggests not to use class variables (or whatever it suggests—I honestly never used it since it brings more harm than help IMSO.)

In your first snippet you have missed the @. The correct code would be:

class A
# ⇓⇓ HERE
  @@test = 10
end
class A
  def test
    @@test # you can access class variable without offense
  end
end
Aleksei Matiushkin
  • 119,336
  • 10
  • 100
  • 160
  • One reason Rubocop says not to use class variables (the ones with two at signs, for clarity of other readers) is that it says class variables are passed by reference when inheriting classes. This means a class can change another’s class variable and that is likely not very obvious or intentional. – Nate Dec 31 '18 at 06:21
  • 1
    @Nate well, this is the desired behaviour for class variables contrarywise to instance variables on the class level, so Rubocop would better shut up (or explain the intent so that people do not need to post questions like above on SO.) – Aleksei Matiushkin Dec 31 '18 at 06:23
  • 1
    @Nate class variables are perfectly usable when one needs to collect some base-class-wide information and keep it accessible from the whole hierarchy tree branch. For instance, `ActiveRecord::Base` could have all the instances, direct and indirect, stored _in the class variable_. That is what class variables were invented for. – Aleksei Matiushkin Dec 31 '18 at 06:25
  • 2
    @Nate's right about the intent from their documentation, but Aleksei's right about everything else. It's unreasonable for RuboCop to oust class variables and provide a fake correction. – RWDJ Dec 31 '18 at 06:44
  • @AlekseiMatiushkin, I didn't miss a `@`. That's the exact code provided by RuboCop to fix `correction:Style/ClassVars` in the provided [documentation](https://rubocop.readthedocs.io/en/latest/cops_style/#styleclassvars). I knew the second snippet would fail too, but I added it for completeness since ruby made the suggestion. Your response mostly reflects what I'm thinking, so I'll check back in a day or so and accept your answer if no one else figures out if RuboCop just has a typo in their documentation or if they're just straight wrong. But, good insight into the class method. – RWDJ Dec 31 '18 at 06:49
  • 1
    @RWDJ if you do care about Rubocop docs, create an issue in their repo; it makes more sense than sitting here waiting for strangers wild guesses :) – Aleksei Matiushkin Dec 31 '18 at 06:51
  • @AlekseiMatiushkin, true enough. I actually did that originally and was directed to SO, but I just posted a ticket on their style guide repo. Anyways, I'd still like to see what people come up with for resolving it, since the docs definitely do not. Your suggestion to create a class method to access the class instance variable works fine and it's reasonable, although I would still never do that unless I was forced to, so I'll accept your answer. – RWDJ Dec 31 '18 at 07:23
  • For what it’s worth, I’ve been taught for years not to use the @@variables. If you have any, it’s still good practice to get rid of them, though honestly I don’t have a great reason why other than there are other not-as-fragile ways to store variables. – Nate Dec 31 '18 at 14:26
  • 1
    @Nate once again, there is no better way to store anything shared between subclasses. – Aleksei Matiushkin Dec 31 '18 at 14:56
  • 3
    Sure there is. Use a class-level instance variable as the cop suggests. This gets shared amongst all subclasses, but doesn’t leave you with the possibility of a child class changing the parent’s variable on accident. If you need the instances to have access to your class instance variable, you likely should be using a reader method anyways. If you use Rails, you can also check out `class_attribute` which has a lot of inheritance features that are too much for this comment. – Nate Dec 31 '18 at 14:59
  • 1
    I recommend this question and answer for understandind what cops suggets: https://stackoverflow.com/questions/15773552/ruby-class-instance-variable-vs-class-variable – Foton Mar 02 '21 at 09:54
  • So, @Nate, what is the recommended way if I DO want the child class changing the parent's variable, specifically the registration pattern like ActiveRecord::Base? Turn off the cop? – Honza May 10 '21 at 20:53
7

At the beginning of 2023, the problem is still relevant. Because the rubocop documentation is not the place to post information about the intricacies of OOP in ruby.

The dislike of using class variables comes from unexpected behavior when we use class inheritance. But we love to watch the code, not read the description, and the documentation clearly says:

You have to be careful when setting a value for a class variable; if a class has been inherited, changing the value of a class variable also affects the inheriting classes. This means that it's almost always better to use a class instance variable instead.

I would like to supplement Alexey Matyushkin's answer and show the behavior of class variables with simple examples. And also explain what this can lead to.

I confirm that the code from the rubocop documentation is some kind of nonsense:

# good
class A
  @test = 10
end

class A
  def test
    @@test # you can access class variable without offense
  end
end

class A
  def self.test(name)
    class_variable_get("@@#{name}") # you can access without offense
  end
end

begin
  puts A.new.test
rescue => e
  puts e.message
end

begin
  puts A.test 'test'
rescue => e
  puts e.message
end

puts "RUBY_VERSION: #{RUBY_VERSION}"

=>>>

uninitialized class variable @@test in A
Did you mean?  @test
uninitialized class variable @@test in A
Did you mean?  @test
RUBY_VERSION: 2.5.3

What rubocop really wanted to tell us.

puts 'When we use "classic" class variables:'
class A
  @@var = 10
  cattr_accessor :var
end
class Aa < A
end
puts Aa.var, '- the child class has inherited the methods and the value of the variable.'
Aa.var = 20
puts A.var, '- but the variable of the parent class was implicitly changed (bad)!'

puts 'When we use class instance variables:'
class B
  @test = 10
  class << self
    attr_accessor :test
  end
end
class Bb < B
end
puts Bb.test, '- the child class has inherited the methods, but not the value of the variable (this is also bad)!'
Bb.test = 20
puts B.test, '- a change in the child class does not lead to a change in the parent.'

=>>>

When we use "classic" class variables:
10
- the child class has inherited the methods and the value of the variable.
20
- but the variable of the parent class was implicitly changed (bad)!
When we use class instance variables:

- the child class has inherited the methods, but not the value of the variable (this is also bad)!
10
- a change in the child class does not lead to a change in the parent.

And what's the big deal? How can this harm?

One way to modify BIG programs is to inherit the class and make your own changes to it. Often the project is complex, there are many implicit dependencies (let's be honest =)), and if you make changes directly to the class, the project will crash. Therefore, we use inheritance, the child class is used in a new service with its own settings, or the child class changes the behavior of one part of the program. And if, during inheritance, the child class suddenly changes the base class, then inheritance loses its meaning! Flexibility is lost.

But any question needs to be viewed in context. If you are writing a miniature project alone, then there is nothing wrong with @@ var. You just need understanding.

Zlatov
  • 126
  • 1
  • 6
  • The issue is, there are times and places where this "unexpected behavior" isn't unexpected at all, and is the exact behavior that is desired, hence someone is choosing to use a class variable. Getting a warning for using a language feature for its intended purpose, and in its intended way, doesn't seem like something that should raise alarm bells by an overly-opinionated linter. – ForeverZer0 Aug 15 '23 at 04:06
0

Condensed version of the top answer:

class A
  @test = 10
  def test
    @test # the instance's instance variable, which will be nil
    class.instance_variable_get(:@thing) # the class's instance variable, which you set to 10
  end
end
Mirror318
  • 11,875
  • 14
  • 64
  • 106