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

Constants vs. class methods

Open jibiel opened this issue 11 years ago • 16 comments

Greetings, I would like to hear some community standpoint regarding this matter.

class User < ActiveRecord::Base
  ROLES = %w(admin moderator contributor)
end

# > User::ROLES
# => ["admin", "moderator", "contributor"]

or...

class User < ActiveRecord::Base
  self.roles
    @roles ||= %w(admin moderator contributor)
  end
end

# > User.roles
# => ["admin", "moderator", "contributor"]

I personally prefer class methods since this approach looks more object-oriented to me while screaming snake cased constant feels kinda ugly typographically.

There is also interesting Stack Overflow discussion.

jibiel avatar Jul 07 '14 13:07 jibiel

I usually define it as a const, and then "alias" it as a class method like so:

class User < ActiveRecord::Base
  ROLES = %w(admin moderator contributor)

  def self.roles
    self::ROLES
  end
end

I feel this establishes the intent that roles are constant, while avoiding the "screaming snake case."

TSMMark avatar Jul 07 '14 14:07 TSMMark

This isn't really a discussion of "constants" versus "class methods". It is more like "public class constants" versus "class methods that return a constant" :grinning:

I typically wouldn't have a public class constant. I might have a constant on a module, like so:

module Foo
  ROLES = %w(admin moderator contributor)

  class User
    # ... snip ...
  end
end

But that is also assuming that you're not using Rails or something that discourages the use of modules. Also, as evidenced by the discussion in this Stack Overflow answer, the best solution is going to be very situational.

lee-dohm avatar Jul 07 '14 14:07 lee-dohm

I prefer the Class method way too. maybe we should think about what’s the difference between two of them on How Ruby implemented with C codes behind this. in the other words, which way is better performance.

On Jul 7, 2014, at 10:43 PM, Mark Allen [email protected] wrote:

I usually define it as a const, and then "alias" it as a class method like so:

class User < ActiveRecord::Base ROLES = %w(admin moderator contributor)

def self.roles self::ROLES end end I feel this establishes the intent that roles are constant, while avoiding the "screaming snake case."

— Reply to this email directly or view it on GitHub.

metrue avatar Jul 07 '14 14:07 metrue

Well, as somewhat expected any of the wrapper methods is slower than the simple constant:

                                 user     system      total        real
Just constant:               0.010000   0.000000   0.010000 (  0.010488)
Constant wrapper:            0.020000   0.000000   0.020000 (  0.019495)
Instance variable caching:   0.030000   0.000000   0.030000 (  0.021562)
No constant, no caching:     0.070000   0.000000   0.070000 (  0.072152)

...at least two times.

jibiel avatar Jul 07 '14 19:07 jibiel

While the benchmark is a striking argument, I usualy go back to this blog entry by @avdi to remind myself of the trouble I repeatedly ran into when using constants before.

Let me quote him quickly:

  • [...] I often find myself having to convert a constant to a method when I realize that it needs to be a dynamically calculated value. Why didn’t I just make it a method returning a constant value in the first place?
  • Constants lead to constant redefinition warnings in dynamically-reloaded code unless you are careful.
  • Methods are easy to stub out in tests. Constants are a pain to stub out. [...]
  • Methods are easy to override on a per-object basis with singleton method definitions or module insertion. Constants are not.
  • Finally, they are just an extra thing to remember. Was that default value a public constant, or a public class method? I can’t remember, let me go check the docs…

As for the benchmark, I assume a constant wrapper might be a reasonable trade-off. It does not offend all reasons listed above, but is fast enough still. Personally, I always prefer methods.

halo avatar Jul 31 '14 11:07 halo

Wow, thanks, reference to @avdi is a pretty damn good argument for me. Despite the benchmark (just trying to be somewhat objective) I still think wrapper methods rock.

jibiel avatar Jul 31 '14 11:07 jibiel

I like methods better. I used them instead of constants in some recent code, though, and at least two people asked me why I didn't use a constant instead. It may take people some time to get used to the new style if this makes it into the style guide.

michaelstalker avatar Jan 05 '15 19:01 michaelstalker

@jibiel: I know this is an old issue, but might you still have your benchmark code (for above sighted results) laying around? If so could you share it? I would like to play with it a bit.

dekellum avatar Jun 22 '16 23:06 dekellum

@dekellum Here you go:

require 'benchmark'

n = 100000

class User
  ROLES = %w(admin moderator contributor)

  def self.roles_constant_wrapper
    self::ROLES
  end

  def self.roles_with_caching
    @roles ||= %w(admin moderator contributor)
  end

  def self.roles
    %w(admin moderator contributor)
  end
end

Benchmark.bm(26) do |x|
  x.report('Just constant:')             { 1.upto(n) do; User::ROLES; end }
  x.report('Constant wrapper:')          { 1.upto(n) do; User.roles_constant_wrapper; end }
  x.report('Instance variable caching:') { 1.upto(n) do; User.roles_with_caching; end }
  x.report('No constant, no caching:')   { 1.upto(n) do; User.roles; end }
end

jibiel avatar Jun 23 '16 07:06 jibiel

Not at laptop but what were the results

tommydangerous avatar Jun 23 '16 14:06 tommydangerous

That benchmark is slightly unfair to the constant wrapper, as both plain ROLES and User::ROLES are faster than self::ROLES. Surprisingly, wrapped User::ROLES is actually faster than wrapped ROLES and wrapped ::User::ROLES.

booch avatar Jun 23 '16 20:06 booch

                                 user     system      total        real
Just constant:               0.000000   0.000000   0.000000 (  0.004568)
Constant wrapper:            0.010000   0.000000   0.010000 (  0.009566)
Instance variable caching:   0.010000   0.000000   0.010000 (  0.011220)
No constant, no caching:     0.040000   0.000000   0.040000 (  0.041285)

tommydangerous avatar Jun 23 '16 20:06 tommydangerous

Thanks for sharing @jibiel

@booch I think that is explained by method call overhead (on top of the various forms of constant lookup overhead).

My case, and how I came to this issue, is that I have a Parser where I need to predefine/compile a bunch of lexer Regexp's which are used repeatedly and access performance is important. However I also want it to be possible for users to "override" these Regexp's. I considered wrapper methods but also discovered the self::CONSTANT pattern as an alternative approach. Here is my modified benchmark:

https://gist.github.com/dekellum/e6aff483c8d6b208f281bf5d92c113e6

                                 user     system      total        real
C1.use_direct:               1.140000   0.000000   1.140000 (  1.143424)
C1.use_via_self:             1.620000   0.000000   1.620000 (  1.623499)
C1.use_wrappers:             1.660000   0.000000   1.660000 (  1.669075)
C2.use_via_self:             2.290000   0.000000   2.290000 (  2.282709)
C3.use_wrappers:             2.410000   0.000000   2.410000 (  2.410425)

So for me I think using real-constants and the self::CONSTANT access pattern is best because:

  1. It supports constant "override" in a subclass
  2. It is (slighty) faster than wrapper methods
  3. It is a lot more succinct than wrapper methods

dekellum avatar Jun 23 '16 22:06 dekellum

Oops, my benchmark was a bit buggy but corrected results lead me to same conclusion to use constants accessed via self::CONSTANT

                                 user     system      total        real
C1.use_direct:               1.110000   0.000000   1.110000 (  1.107700)
C1.use_via_self:             1.630000   0.000000   1.630000 (  1.631611)
C1.use_wrappers:             1.690000   0.000000   1.690000 (  1.694374)
C2.use_via_self:             1.580000   0.000000   1.580000 (  1.581633)
C3.use_wrappers:             1.640000   0.000000   1.640000 (  1.640261)

dekellum avatar Jun 23 '16 22:06 dekellum

I've updated the result here https://gist.github.com/benoittgt/ee69d941ac2158d47f701656b58ad99c

$ ruby -v                                                                                                                                                                           
ruby 2.4.0p0 (2016-12-24 revision 57164) [x86_64-darwin14]

# Warming up --------------------------------------
Just constant:                        21.000  i/100ms
Constant wrapper:                     10.000  i/100ms
Instance variable caching:             9.000  i/100ms
No constant, no caching:               4.000  i/100ms
Just constant freeze:                 22.000  i/100ms
Constant wrapper freeze:               9.000  i/100ms
Instance variable caching freeze:      7.000  i/100ms
No constant, no caching freeze:        4.000  i/100ms

Calculating -------------------------------------
Just constant:                       218.701  (± 5.5%) i/s -      1.092k in   5.008328s
Constant wrapper:                    105.221  (± 4.8%) i/s -    530.000  in   5.051556s
Instance variable caching:            93.146  (± 4.3%) i/s -    468.000  in   5.034547s
No constant, no caching:              48.856  (± 4.1%) i/s -    244.000  in   5.000287s
Just constant freeze:                221.771  (± 4.5%) i/s -      1.122k in   5.068586s
Constant wrapper freeze:              93.528  (± 4.3%) i/s -    468.000  in   5.014071s
Instance variable caching freeze:     82.063  (± 3.7%) i/s -    413.000  in   5.040944s
No constant, no caching freeze:       44.045  (± 4.5%) i/s -    220.000  in   5.005072s

Comparison:
Just constant freeze::               221.8 i/s
Just constant::                      218.7 i/s - same-ish: difference falls within error
Constant wrapper::                   105.2 i/s - 2.11x  slower
Constant wrapper freeze::             93.5 i/s - 2.37x  slower
Instance variable caching::           93.1 i/s - 2.38x  slower
Instance variable caching freeze::    82.1 i/s - 2.70x  slower
No constant, no caching::             48.9 i/s - 4.54x  slower
No constant, no caching freeze::      44.0 i/s - 5.04x  slower

I'm still impress about the difference between result from a constant and result from a method content if this benchmark can be read as relevant.

benoittgt avatar Jan 17 '17 22:01 benoittgt

Well, in light of the results I guess we can recommend plain old constant, plus an optional method wrapper. I think that's what most people are doing anyways (at least based on my subjective observations).

bbatsov avatar Jun 12 '19 21:06 bbatsov