solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 229 forks source link

Allow all valid symbols to be names of attributes #116

Closed blatyo closed 11 years ago

blatyo commented 11 years ago

I'm trying to define an object to represent a torrent file. Unfortunately, the keys are named things like "created by" and "announce-list". The issue is that those are not valid @variable names, so the code blows up.

I was trying to do something like this:

class Torrent
  include Virtus

  attribute :"created by", String
  alias_method :created_by, :"created_by"
end

The first goal is that Torrent can accept a hash with "created by" as the key. The second goal is that to_hash produces a hash with "created by" as the key. Let me know if there is a better way.

solnic commented 11 years ago

We can only accept names that are valid method names. In your case you need a mapper that would translate those hash keys into acceptable names.

greyblake commented 11 years ago

@blatyo You might need to do something like this:


class Torrent
  include Virtus

  attribute :created_at   , String
  attribute :announce_list, Array

  MAPPING = {
    :'created at'    => :created_at,
    :'announce-list' => :announce_list
  }

  def initialize(attrs = {}, *args)
    attrs = attrs.dup
    MAPPING.each do|from, to|
      attrs[to] = attrs.delete(from) if attrs.has_key?(from)
    end
    super(attrs, *args)
  end

  def to_hash(*args)
    hash = super
    MAPPING.each do|from, to|
      hash[from] = hash.delete(to)
    end
    hash
  end
end

torrent = Torrent.new(:'created at' => "Once", :'announce-list' => ["one", "two"])
torrent.to_hash  # => {:"created at"=>"Once", :"announce-list"=>["one", "two"]}
blatyo commented 11 years ago

"created by" is a valid method name.

class Object
  define_method :"created by" do
    puts "created by"
  end
end

send "created by" # prints "created by"

Most libraries that provide a similar DSL to Virtus (I'm thinking of the mongo libraries) tend to have an option like :as that allows you to override the name of the accessor methods. An example of the usage would be:

class Torrent
  include Virtus

  attribute :"created by", String, as: :created_by
end

t = Torrent.new(:"created by" => "Bob")
t.created_by #=> "Bob"
t.to_hash #=> {:"created_by" => "Bob"}

When Virtus is used as part of data mapper, how would you define your class to support legacy column names?

I can understand not wanting to support that and a mapper would certainly work. However, if that is the case, then I think it makes sense to have attribute :"created by" fail immediately on load rather than at usage, which is happening now. Doing so would allow a less cryptic error than "@created by is not a valid variable name" (quoted from memory, may not be exact) and a relevant stack trace pointing to the attribute declaration, rather than the initial usage of the object.

Also, I fixed my example. The second argument in alias_method should have been :"created by".

class Torrent
  include Virtus

  attribute :"created by", String
  alias_method :created_by, :"created by"
end

Hope that all makes sense.

solnic commented 11 years ago

I don't like that other similar libs do that. This is a typical use case where the mapper pattern should be used. This way you put too many responsibilities into one object.