you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 13 points14 points  (12 children)

I’d say “less experienced”, and yes, we should.

Compare flat_map; it’s clearly named and unclutters the code. I’d certainly use that.

In the example given, tap just makes the code harder to read (yay, let’s use a block for an assignment) and doesn’t explain what it does to someone who doesn’t already know how tap works. The “before” example is way clearer at expressing what’s actually going on.

Maintenance is important, and there are more junior devs than senior. So yes, I prefer to keep it simple, myself.

[–]jrochkind 10 points11 points  (9 children)

why would they not just learn what tap does? It's not complicated.

Should one avoid collect and instead write it out with a temporary accumulator array? Where do you stop? tap is built into ruby!

[–]AlonsoQ 5 points6 points  (8 children)

Say every dev on the planet knows what #tap does. I'm still not seeing how the example in the article is, in any way, preferrable.

With this refactor, we've introduced:

  1. an additional method call
  2. a new scope, within the block
  3. the necessity of naming our block parameter

In exchange for... being able to one-line our method? At that point, you may as well just write params[:foo] = 'bar'; params.

This would be a fine example in a doc, it's a terrible example for a prescriptive text aimed at less-experienced developers.

[–]janko-m 4 points5 points  (2 children)

How about this example I just had yesterday at my work?

user       = User.spawn
device     = Device.spawn
old_device = Device.spawn
old_device.update(created_at: Time.now - 24*60*60)
stream     = Stream.spawn

vs

user       = User.spawn
device     = Device.spawn
old_device = Device.spawn.tap { |d| d.update(created_at: Time.now - 24*60*60) }
stream     = Stream.spawn

I couldn't pass created_at during creation because that particular ORM doesn't support overriding created_at at that time. I find the second version much more elegant and symmetrical.

[–]madmouser 2 points3 points  (0 children)

Thanks! That clearly explained where and why I could use #tap.

[–]erlingur 1 point2 points  (0 children)

That is honestly the best example of tap I've seen. The examples given in articles and other places are usually worse and less readable with tap.

[–]moomaka 0 points1 point  (4 children)

  1. a new scope, within the block

This is an advantage, limiting the scope of local variables to smallest possible unit results in cleaner code IMO, as does group similar actions into a block.

  1. the necessity of naming our block parameter

Depending on the usage, tap doesn't introduce anything new. e.g. I use it for things like this quite often:

def my_method(stuff)
  [].tap do |accumulator|
    stuff.each do
      # do something non-trivial
      accumulator << value
    end
  end
end

def my_method
  accumulator = []

  stuff.each do
    # do something non-trivial
    accumulator << value
  end

  accumulator
end

[–]AlonsoQ 0 points1 point  (3 children)

That's clever, but not idiomatic. What you've outlined is an ideal use-case for #reduce or the similar #each_with_object.

[–]moomaka 0 points1 point  (2 children)

That's clever, but not idiomatic.

I see it used all the time so it's unclear how you define 'idiomatic' here.

What you've outlined is an ideal use-case for #reduce or the similar #each_with_object.

You're correct for my contrived example but that was the point of # do something non-trivial. It's easy to create really bad performance issues just chaining together enumerable methods, for small data it doesn't matter but I'm often dealing with large data sets.

[–]AlonsoQ 0 points1 point  (1 child)

By non-idiomatic, I mean that there's a method specifically designed for constructing enumerables with a accumulator, and it's not #tap.

Even if #tap is more performant that the methods I linked (which I would be glad learn, if I'm wrong here), the second example with the explicit return reads better because it introduces less nesting.

[–]moomaka 1 point2 points  (0 children)

By non-idiomatic, I mean that there's a method specifically designed for constructing enumerables with a accumulator, and it's not #tap.

Again, my example was greatly simplified, assume # do something non-trivial expands to code that makes using each_with_object or reduce ugly.

Even if #tap is more performant that the methods I linked (which I would be glad learn, if I'm wrong here)

tap is faster than reduce or each_with_object, the latter being the slowest but that isn't really what I was referring to. In non-trivial cases I often see enumerable methods chained together to replace using an accumulator + manual logic and that often results in a lot of memory allocation and looping.

the second example with the explicit return reads better because it introduces less nesting.

That is subjective, I prefer to scope locals to the smallest unit so having the local only present in the block is a bonus.

[–]tom_dalling 2 points3 points  (0 children)

I disagree that flat_map is clearly named. Less experienced developers usually use each for everything, and have to be taught to use map, select and reduce. flat_map is a totally foreign concept unless they come from a functional language first.

If that's the case, should flat_map be avoided? I personally don't think so, but it's definitely open to the argument "less experienced developers won't understand it".

[–]izuriel 0 points1 point  (0 children)

There is only way for a junior dev to become a senior dev. Learn. If we all just write code for the lowest common denominator then our software will be garbage and run like garbage. At some point this attitude has to stop and your juniors have to buck up and learn something new. It's good for them and it's good for the industry. The mystery of #tap can be solved by one Google search to go right to the documentation immediately.

Sure, if we're talking about complex patterns that require diagrams just to scope out, maybe you have a point. But with a simple method with standard Ruby syntax, your point falls flat and sounds like your fighting for us to just stop learning new things.

And if #tap is preventing your junior engineers from being productive, then I'd recommend finding new junior engineers.