Should still able to set other reflection successfully

[wendy-clio]

The error caught my attention while implementing another ticket.

Making sure the PolymorphicForeignAssociationExtension will be compatible with any reflection (in this case, ActiveRecord::Reflection::ThroughReflection)

Since this is such a minor fix, will not bump the version but re-release after the merge.

[apalmblad]

What is other? Can we be more specific?

[wendy-clio]

I specify one example underneath in it aka ActiveRecord::Reflection::ThroughReflection

[apalmblad]

Hmm. My general feeling is that the use of try is a bit lazy => why not simply have those methods defined but return nil to that we end up with reflections continuing to look similar, versus needing to remember that we need to guard against some differences.

[wendy-clio]

you are more suggesting to something like this?

def foreign_integer_type(reflection)
  if reflection.respond_to? (:foreign_integer_type)
    reflection.foreign_integer_type
  else
    nil
  end  
end

or simple

def foreign_integer_type(reflection)
  reflection.try(: foreign_integer_type)
end

which feel like the same though

[wendy-clio]

Also, I do not want to just adding moreattr_accessor to ThroughReflection in here https://github.com/clio/polymorphic_integer_type/blob/master/lib/polymorphic_integer_type/extensions.rb#L5-L8 in case there is more reflection I am not aware of.

[MrJakeReid]

I've read Adam's suggestion and while I wouldn't be opposed to it, I don't inherently see the value in adding a method definition to guard against the edge case that's demonstrated in the specs. I think checking with try is acceptable, and if there are more edge cases then I'd be open to the refactor.