custom helper preprocesor about active_importer HOT 4 CLOSED

Although this seemed nice at first, on a second thought I think this is just syntactic sugar. And while that alone is not bad (syntactic sugar can be good), I think in this case there's little benefit in return. You could also write something like this:

class ProductImporter < ActiveImporter::Base
  imports Product

  column('Category', :category) { |v| normalize(v) }
  column('Section', :section) { |v| normalize(v) }

  ...  

  private

  def normalize(value)
     value.strip.downcase
  end
end

We could even make the following notation work too, if it's not already:

  column 'Category', :category, -> (v) { normalize(v) }
  column 'Section', :section, -> (v) { normalize(v) }

I'd even say this is more clear to a new user coming to the library. Your proposal would need people read that feature's documentation, or maybe infer what's happening from the names of things, and trust that what they're inferring is correct.

Would you consider this an acceptable alternative @lporras?

from active_importer.

BTW, I should mention that if what you're doing in some of these helpers is merely invoking a method to the actual value, I suspect this notation is also available:

  column 'Category', :category, &:downcase

That would be equivalent to:

  column('Category', :category) { |v| v.downcase }

It's not applicable to your case above because normalize invokes two methods in sequence to the value of the column, but it might be worth knowing for some cases where you wouldn't even need to declare the helper function to begin with.

from active_importer.

@gnapse, thanks for your quick response, I'm going to test using this:

&:downcase

notation, and if it works then I think i could add the normalize method to the String class and leave it like this:

column 'Category', :category, &:normalize

and about the question related to syntactic sugar, I really like this way you proposed:

column 'Category', :category, -> (v) { normalize(v) }
column 'Section', :section, -> (v) { normalize(v) }

from active_importer.

@lporras I would strongly advise against this:

add the normalize method to the String

Monkey-patching is generally being avoided now more than before. It sounds like a good idea, but you can end up polluting the namespace of a class just for too little in return. I stress the idea that the gain in a slightly cleaner and shorter syntax is not enough reason to do it.

At the very least I'd urge you to use the newer Ruby language feature called refinements. It allows you to limit a monkey-patching to just inside the scope inside a specific module. Code outside that module is not affected.

from active_importer.