Rails Ruby on Rails → 3.1

Comments and changes to this ticket

• 

  Michael Koziarski September 2nd, 2008 @ 08:13 AM

  • Assigned user set to “Michael Koziarski”

• You flagged this item as spam.
  

  Joel November 17th, 2008 @ 11:27 AM

  This looks pretty neat, will this trigger changed? on the attribute (which composed_of does not seem to do atm)

• You flagged this item as spam.
  

  Joel November 17th, 2008 @ 04:01 PM

  sigh ignore my comment ^^

• 

  Eloy Duran November 17th, 2008 @ 07:29 PM

  Updated the patch for current HEAD.

  @Joel: I assume you found out that it does in fact trigger the "dirty" status?

• You flagged this item as spam.
  

  Joel November 18th, 2008 @ 04:09 AM

  Indeed I did.

  I had missed a commit of a plugin that busted it tho =)

• 

  Eloy Duran December 5th, 2008 @ 12:26 PM

  Updated for current HEAD

• 

  Eloy Duran December 5th, 2008 @ 12:26 PM

  And this time with a patch :-)

• You flagged this item as spam.
  

  Pascal Ehlert December 8th, 2008 @ 08:34 AM

  In the example above, you say .to_s is for form helpers.

  I couldn't check it out, yet, but is that true? The problem I've always had with my own solution to this problem is that form helpers were actually using attribute_before_typecast for reasons I never fully understood so that all formatting was gone.

  Anyway, this looks nice and much more sophisticated than the existing solution so +1 for the patch.

• 

  Eloy Duran December 8th, 2008 @ 09:00 AM

  @Pascal: You are right, so there is indeed an implementation of the foo_before_type_cast method (where foo' is the decorator), which calls the #to_s method on the decorator instance to get the value for the form field.

• 

  Jeremy Kemper December 8th, 2008 @ 08:33 PM

  • Milestone cleared.
  • State changed from “new” to “open”

  Nice! The API feels a bit wordy, though, like a specification rather than a declaration.

  Consider

  
  attribute_decorator :date_of_birth, :class => CompositeDate, :decorates => [:day, :month, :year]
  

  versus

  
  compose :date_of_birth, :of => [:day, :month, :year], :as => CompositeDate
  

  or

  
  view :date_of_birth, :as => ComposedDateAttribute.new(:day, :month, :year)
  

• 

  Manfred Stienstra December 9th, 2008 @ 10:34 AM

  Jeremy, your compose example doesn't read really well when you're decorating one attribute. I guess we can meet halfway? How about this:

  
  class Artist < ActiveRecord::Base
    view :location, :as => Location, :decorating => :gps_location
    view :date_of_birth, :as => CompositeDate, :decorating => [:day, :month, :year]
  
    validate_view :location
    validate_view :date_of_birth, :message => 'is not a valid date'
  end
  

• 

  Eloy Duran December 10th, 2008 @ 08:48 PM

  Updated the API according to what Manfred suggested, did some cleaning and updated for edge.

• 

  Eloy Duran December 10th, 2008 @ 09:03 PM

  Forgot to update a deprecation warning for composed_of.

  • attribute_decorator.diff 85.8 KB

• You flagged this item as spam.
  

  Brennan Dunn December 10th, 2008 @ 10:42 PM

  What about aiming toward something a bit more like this plugin I recently wrote: http://github.com/brennandunn/at...

  The reason being is that I think, for the purpose of adding helper functionality to attributes, the extension model implemented by AR's association proxy/named_scope is more concise and easy to work with.

  Example:

  
    class Person < ActiveRecord::Base
    
      with_attribute :name do
      
        def has_middle_name?
          self.split(' ').size == 3
        end
      
        def surname
          self.split(' ').last
        end
      
      end
    
    end
  
    @person = Person.create :name => 'John C. Doe'
    
    @person.name.has_middle_name?  # => true
    @person.name.surname           # => "Doe"
    
    # and of course, 
    @person.name                   # => "John C. Doe"
  
  

  Thoughts?

• 

  Manfred Stienstra December 11th, 2008 @ 02:49 PM

  Yesterday, we were talking to Jeremy on IRC and I promised to show some examples of how the API would function.

  In short, we're trying to find a way to compose attributes on an ActiveRecord class in such a way that it supports:

  1. Conversions: Attributes <=> Value object <=> Form data
  2. Validation

  Jeremy raised two problems: using existing classes and assigning instances through the attribute writer. Basically the following:

  
  class Artist < ActiveRecord::Base
    view :date_of_birth, :as => DateDecorator, :decorating => [:day, :month, :year]
  end
  
  artist = Artist.new(:date_of_birth => '24-05-1980')
  artist.date_of_birth += 10.days
  

  From what I understand he thought that meant changing the API slightly and have an object do all the conversions:

  
  view :date_of_birth, :as => ComposedDateAttribute.new(:day, :month, :year)
  

  The conversion class could look something like this:

  
  class ComposedDateAttribute
    attr_accessor :attributes
    
    def initialize(*attributes)
      @attributes = attributes.flatten
    end
    
    # Attributes => Value object
    def instantiate(record)
      Date.new(*attributes.reverse.map { |attribute| record.send(attribute) })
    end
    
    # Value object => Form data
    def form_value(date)
      date.strftime("%Y-%m-%d")
    end
    
    # Form data => Value object
    def parse(string)
      Date.parse(string)
    end
    
    # Value object => Attributes
    def attributes(date)
      [date.year, date.month, date.day]
    end
    
    # Validation
    def valid?(string)
      string =~ /\d{4}-\d{2}-\d{2}/
    end
  end
  

  The upside is that you get an actual Date object, the downside is that it looks like crap and becomes even weirder if you want to use the same class as the value object. Maybe I misunderstood Jeremy's intentions?

  As far as I can see this can be solved with the current implementation:

  
  class DateDecorator < Date
    
    # Attributes => Value object
    def initialize(day, month, year)
      super(year, month, day)
    end
    
    # Value object => Form data
    def to_s
      strftime("%Y-%m-%d")
    end
    
    # Form data => Value object
    def self.parse(value)
      case value
      when Date
        new(value.day, value.month, value.year)
      when CompositeDate
        value
      else # Assume it's a String
        day, month, year = value.scan(/(\d\d)-(\d\d)-(\d{4})/).flatten.map { |x| x.to_i }
        new(day, month, year)
      end
    end
    
    # Value object => Attributes
    def to_a
      [day, month, year]
    end
    
    # Validation
    def valid?
      day != 0 && month != 0 && year != 0
    end
  end
  

  The upside of this solution is that the class makes sense: it's an actual decorator. The downside is that strict class checking code might break. I guess the method names should change to be more expressive so this use case makes more sense.

  Finally, Jeremy says that he "[doesn't] think it should be the value object's responsibility to understand how it's composed from the database." I completely agree, that's why we use a decorator object to sit 'around' the value object to facilitate the conversions.

• 

  Jeremy Kemper December 11th, 2008 @ 09:45 PM

  Combining the value object and it's parse/dump behavior is more confusing to me. It's a poor separation of concerns that makes working with existing classes cumbersome.

  I suggest

  
  class ComposedDateView
    def initialize(year, month, day) @y, @m, @d = year, month, day end
  
    def load(record)
      Date.new(record.send(@y), record.send(@m), record.send(@d))
    end
  
    def parse(string)
      Date.parse(string)
    end
  
    def dump(date)
      { @y => date.year, @m => date.month, @d => date.day }
    end
  end
  

  Then we can do nice things like share attribute types as plugins, subclass composers to change behavior without having to change the value object code, and refactor YAML serialization as a view.

  The form value and validation stuff aren't really necessary. We can already handle that with ValueObject#to_s and existing model validations.

• 

  Jeremy Kemper December 11th, 2008 @ 09:53 PM

  YAML example:

  
  class YamlView
    def initialize(attr) @attr = attr end
  
    def load(record)
      parse(record.send(@attr))
    end
  
    def parse(string)
      YAML.load(string) if string
    end
  
    def dump(object)
      { @attr => is_yaml?(object) ? object : YAML.dump(object) }
    end
  
    protected
      def is_yaml?(object)
        object.is_a?(String) && object =~ /\A---/
      end
  end
  

• 

  Manfred Stienstra December 12th, 2008 @ 10:41 AM

  I have to admit, when you limit the responsibility of the view class to just loading, parsing, and dumping it looks ok.

  I don't agree on the ValueObject#to_s thing though, form helpers currently use #{attribute_name}_before_type_cast on the record instead of to_s to get the value to put back into the form on validation errors. So that still has to be addressed somehow.

  If you like, I can set up an example project to explore your proposal.

• 

  Eloy Duran December 13th, 2008 @ 11:02 AM

  @Manfred: I think that'd be a good idea. Maybe include our real use case that it all was based on?

• 

  Michael Koziarski February 1st, 2009 @ 02:04 AM

  • Assigned user changed from “Michael Koziarski” to “Jeremy Kemper”
  • Milestone set to “2.x”

  Removing from 2.3, I'd still like to get this in but we're close enough now that this shouldn't be 'blocking'

  Assigning to jeremy

• 

  Manfred Stienstra May 26th, 2009 @ 11:33 AM

  • Milestone cleared.

  I've been working on this again as part of a new application. I've been abstracting some of the composed, formatted and decorated attribute accessors. Now I basically do the following:

  views :fee, :as => Fee.new(:fee_in_cents)
  

  class Fee < ActiveRecord::AttributeView
    def load(fee_in_cents)
      if fee_in_cents.blank? or fee_in_cents.zero?
        ''
      elsif (fee_in_cents % 100).zero?
        "%d" % (fee_in_cents / 100)
      else
        "%.2f" % (fee_in_cents / 100.0)
      end
    end
  
    def parse(input)
      return nil if input.blank?
  
      input = input.gsub(/[^\.^\,^\d]/, '')
      whole, cents = input.split(/\,|\./)
      (whole.to_i * 100) + cents.to_i
    end
  end
  

  I've solved the problem with forms and before_type_cast. I'm considering a few things now:

  1. Should we add a format method to the view so you also get a formatted_fee accessor?
     
  2. Should we allow people to specify which accessors get defined (ie. :only_define => [:getter, :formatter])

  Current implementation on: http://gist.github.com/118033

• You flagged this item as spam.
  

  Chris Hapgood June 25th, 2009 @ 05:11 PM

  Manfred's example feels like a natural evolution in Rails. Jeremy hits the key point: it factors out the view behavior into a kind of Presenter, but since it also accommodates reversing the Presenter (to parse input) it might be called an attribute agent. It could allow much of the existing composed_of behavior to remain since the construction of value objects from DB attributes is orthogonal to view behavior (although the :converter option might be merged into these attribute agents).

  Once you've added support for these agents, why not take the next step and instantiate simple agents for EVERY model attribute and eliminate the troublesome call to before_type_cast by the AR form helper altogether? My impression is that before_type_cast has become the de facto presenter for AR attributes and it doesn't feel right.

  You would also expose an ideal entry point for i18n (parsing/presenting of dates, times, floats, etc.).

• 

  Michael Koziarski June 26th, 2009 @ 06:11 AM

  I can't see a reason to allow configuration for the different methods to be defined. If they really care you can always either override them after the fact or undef_method if you want to lose them.

• 

  Jeremy Kemper May 4th, 2010 @ 06:18 PM

  • Milestone set to “3.1”
• 

  Aaron Patterson September 22nd, 2010 @ 10:45 PM

  • Assigned user changed from “Jeremy Kemper” to “Aaron Patterson”
  • Importance changed from “” to “Low”
• 

  Ryan Bigg October 9th, 2010 @ 09:45 PM

  • Tag cleared.

  Automatic cleanup of spam.