I created a Rails Concern for my ActiveModel called List
When I run Product.all
from the Rails Console, I get:
NameError: undefined local variable or method `parameters' for Product:Class
When parameters is changed to @parameters I get this error:
NoMethodError: undefined method `include?' for nil:NilClass
Which is better, using a constant PARAMETERS or @@Parameters? Pros and Cons?
module List
extend ActiveSupport::Concern
require 'csv'
parameters = [
:visible,
:desc,
:value,
]
attr_accessor(*parameters)
def initialize(attributes = {})
attributes.each do |name, value|
send("#{name}=", value)
end
end
def persisted?
false
end
module ClassMethods
def all
list = []
filename = File.join(Rails.root,"app/models/data/#{self.name.downcase}.csv")
CSV.foreach(filename, headers: true) do |row|
list << self.new(row.select{|key,_| parameters.include? key.to_sym })
end
return list
end
def visible
list = []
filename = File.join(Rails.root,"app/models/data/#{self.name.downcase}.csv")
CSV.foreach(filename, headers: true) do |row|
list << self.new(row.select{|key,_| parameters.include? key.to_sym }) if row['visible']=='1'
end
return list
end
end
end
For a quick solution, to make Product.all
and Product.visible
work with the least amount of modification to your existing code, you can define a parameters
method inside module ClassMethods
. For example:
def parameters
@parameters ||= [:visible, :desc, :value]
end
This method solution can also serve as a long-term solution if you plan to use the parameters outside of the concern, or if a subclass might want to define its own parameters.
However, if the parameters are only meant to be used inside this concern, and this data will never change, at least not through any application logic, then a constant would be the best solution because it conveys the proper meaning to the reader. I would also freeze it to prevent modification:
PARAMETERS = [:visible, :desc, :value].freeze
Another option, as Rich mentioned, is to define a class variable. Note that the constant will work whether you define it inside the List
module, or inside the ClassMethods
module. However, a class variable will only work inside the ClassMethods
module if you want Product
to be able to call it as parameters
.
Also, note that self
is implied in any method within ClassMethods
, so you don't need to specify it. If you defined a parameters
method, it would be considered a Product
class method, and if you used parameters
within the all
method, it would refer to the class method, not an instance method as suggested by Rich.
Class variables are generally discouraged in Ruby because their side effects are often misunderstood. The Ruby Style Guide recommends avoiding them: https://github.com/bbatsov/ruby-style-guide#no-class-vars
As for speed, I compared the method and constant solutions, and it looks like the constant is faster:
require "benchmark/ips"
PARAMETERS = [:visible, :desc, :value].freeze
def parameters
@parameters ||= [:visible, :desc, :value]
end
def uses_constant
puts PARAMETERS
end
def uses_method
puts parameters
end
Benchmark.ips do |x|
x.report("constant") { uses_constant }
x.report("method") { uses_method }
x.compare!
end
The result:
Comparison:
constant: 45256.8 i/s
method: 44799.6 i/s - 1.01x slower