Make the default exposure send an empty hash when the resource params are nil #34

Closed
wants to merge 1 commit into from

2 participants

@coffeencoke

If the resource param is nil, it sends to the initialize method as nil. Because of this, every initialize method must handle 3 types of values: default value ({}), nil value, and a hash with given values.

We are not using ActiveRecord, Mongoid, or any other ORM. For each model we use decent exposure with, we have to do the following:

  def initialize(hash={})
    hash ||= {} # do not want to do this!
    self.some_attribute = hash[:some_attribute]
  end
@coffeencoke coffeencoke Make the default exposure send an empty hash when the resource params…
… does not exist

If nil is sent, every initialize method must handle the nil case as well as a default "non nil" case, as well as a hash with values
2263897
@voxdolo

Hey Matt,

Thanks for the pull request. I think we've got a built in way for you to customize that behavior without any library changes already. Classes which have access to the expose method, also have access to the default_exposure method. This allows you to easily do something like this:

class MyController < ApplicationController                                      
  default_exposure do |name|                                                    
    collection = name.to_s.pluralize                                            
    proxy = name.to_s.classify.constantize                                                                                                                          
    if id = params["#{name}_id"] || params[:id]                                 
      proxy.find(id).tap do |r|                                                 
        r.attributes = params[name] unless request.get?                         
      end                                                                       
    else                                                                        
      proxy.new(params[name]||{})                                               
    end                                                                         
  end                                                                           
end

Which I think is essentially what you're after, but customize away :)

Cheers,
Stephen

@voxdolo voxdolo closed this Jan 4, 2012
@voxdolo

Also worth noting, this will be a lot easier to customize in a modular fashion in the 2.0 release... which will happen Real Soon Now.

@coffeencoke
@coffeencoke

I allowed a bit of time for me to think about this and I don't quite understand. This is something I would want for every use of decent exposure, for every project. It's a great gem, but I have found myself using my fork for every new app because of this issue. And I do think it's an issue.

Why would you want to pass nil as the argument into the initializer method? It causes the default arguments to be completely ignored. If you could just give me a bit more feedback as to why this is not something you want, perhaps I can change it to be more acceptable.

@voxdolo

@coffeencoke it's not something I don't want. I've just not found any need for it, since the majority of my uses of it are fronted by an ORM. The change looks fine, actually. My only concern is that nothing explicitly calls out reason for the behavior you propose to exist in the specs. As mentioned previously, work on DE2 is progressing (albeit slowly) and I'd hate to see features left behind because they weren't clearly represented by a spec. If you'd like to add that, I'll be happy to pull this in mainline.

Thanks for your hard work and sticking with it :)

@coffeencoke
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment