madeintandem / hstore_accessor

Adds typed hstore-backed field support to ActiveRecord models.
MIT License
242 stars 47 forks source link

Array should be not be nil but [] and hash should be {} by default #20

Closed andreorvalho closed 9 years ago

andreorvalho commented 10 years ago

Hey guys,

The behaviour I expected when using an array is that if it's empty to return [] and not nil. The same behaviour for hash but returning {}.

I tried to fork it and set a pull request with the following test:

context "when assigning values it" do

  let(:product) { Product.new }
  it "correctly sends an empty array to start with" do
    expect(product.tags).to eq []
  end

  it "correctly sends an empty array to start with" do
    expect(product.reviews).to eq({})
  end
end

When I tried to push a branch I got: git push origin empty_array_instead_of_nil

error: The requested URL returned error: 403 while accessing https://github.com/devmynd/hstore_accessor.git/info/refs fatal: HTTP request failed

andreorvalho commented 10 years ago

is there nobody interested in this? I mean shouldnt that be the actual default behaviour?

thegrubbsian commented 10 years ago

The repo isn't open so you'll have to submit a pull request. If you can fix the problem and include tests we'll get it merged and released.

andreorvalho commented 10 years ago

But maybe I am doing something wrong. but I did was:

should I try again and tell you which commands I used?

crismali commented 9 years ago

I'm not sure this is really the behavior we want. HstoreAccessor generally makes the hstore fields behave like other ActiveRecord attributes and this looks like a deviation from that. If you have an hstore or array type of column in ActiveRecord, the default value is always nil. It seems to me that this sort of thing is best handled in a migration, ie by setting the default for the hstore column itself to be a hash that contains the desired defaults for each HstoreAccessor field.

Alternatively, you could always override the getter to something like:

def my_hstore_accessor_hash_field
  self.my_hstore_accessor_hash_field ||= {}
  super
end
andreorvalho commented 9 years ago

@crismali

If you have an hstore or array type of column in ActiveRecord, the default value is always nil

what is a column of the type array on ActiveRecord? shouldnt that behave like an association? I am not sure I understand what you mean here, can you show me a piece of code on active record that shows this? maybe a test?

It seems to me that this sort of thing is best handled in a migration, ie by setting the default for the hstore column itself to be a hash that contains the desired defaults for each HstoreAccessor field.

Thats not a bad alternative.

But anyways the gem is yours and I was trying to add something that seemed to give a better behaviour if you dont agree is fine. Thanks for your help :)

crismali commented 9 years ago

@andreorvalho you can create an array type of column by doing:

add_column :foos, :bars, :string, array: true

Glad to help, and we're always happy to discuss new features. =)