tap, FP and cleverness

scratch.md

For your consideration

ServiceResult.new(grant_applications: grant_applications)

def create_grant_applications
  grant_program.new_eligible_users
    .map { |user| user.grant_applications.build(grant_program: grant_program) }
    .map { |grant_application| grant_application.save!(skip_validations: true) }
end

#create_grant_applications

Which is better here? Saving off a local variable? Or using tap so that you return the same thing, without the need for a local variable?

local variable:

      eligible_users.map do |user|
        grant_application = user.grant_applications.build(grant_program: grant_program)
        grant_application.save!(skip_validations: true)
        grant_application
      end

Using tap and a block

      eligible_users.map do |user|
        user.grant_applications.build(grant_program: grant_program).tap { |i| i.save! skip_validations: true }
      end

  eligible_users
    .map { |user| user.grant_applications.build(grant_program: grant_program) }
    .each { |grant_application| grant_application.save!(skip_validations: true) }

The last two are attempts to write in a more functional style, calling functions with return values as in puts. If you've worked with a language that has a pipe operator |> or the pipe in bash you'll recognize a similar with syntax like this:

defmodule Acronym do
  @doc """
  Generate an acronym from a string.
  "This is a string" => "TIAS"
  """
  @spec abbreviate(string) :: String.t()

  @word_delimiter ~r/[A-Z]|\b\w/
  def abbreviate(string) do
    string
    |> split_string
    |> Enum.join
    |> String.upcase
  end

  defp split_string(str) do
    Regex.scan(@word_delimiter, str)
  end
end

I think there's times where this approach can be very helpful. Rather than defining a bunch of meaningless intermediate variables you can just list out the data transformations that you need. Active Record supports this pattern very well, returning ActiveRecord relations, which also include the Enumerable module, to allow for maximum chainability:

  def eligible_users
    user_ids = Volunteering::Time
      .select(:user_id)
      .group(:user_id)
      .having("sum(hours) >= ?", 40)
      .approved
      .this_year

    company.users.where(id: user_ids)
  end

I've come to develop an allergy to local variables as a result of all this. But am I fooling myself? In the ruby case above with tap is this just a way for me to feel clever at the cost of readability? As this commentor on reddit puts it

I typically combine tap with inject to get maximum job security

he goes on

sure, lets say you have a regular instance variable, well everyone can understand that so lets replace that with a tap. Nine times out of ten people are just doing that to feel cool about themselves or shave off a few keystrokes. The problem is that code with tap is only partially intention obscuring and we need full obscurity if we want to make sure no one can ever refactor our code.

This is where inject comes in. The first step with using inject is to make sure you aren't using the reduce synonym. Inject makes no sense unless you have a background in smalltalk and it's safe to assume very few people have that. I've seen people use inject before to generate static data structures where they didn't want to repeat a value in a hash that was the same for a few different keys. Really the sky is the limit with inject especially when you ignore it's reason for existing.

tl;dr: i think most uses of tap are just to help people feel like they are smart and/or to obscure the intention of the code. there is also a minority who truly feel like the saved keystrokes matter.

Discuss