Ruby I don't like #2 - catch(:wtf) { throw :wtf }
Published almost 5 years ago

The 1960s and 1970s saw computer scientists move away from GOTO statements in favor of the structured programming programming paradigm. Some programming style coding standards prohibit use of GOTO statements. – Wikipedia

Ruby takes the whole GOTO nonsense to an entirely new heights. Ruby’s version of GOTO/LABEL is called throw/catch. The lunacy goes further as Ruby’s throw is equivalent to GOTO with a return value.

def hello
  throw :done, "wtf"
end

catch(:done) { hello }
=> "wtf"

Not only it makes the flow control hard to follow, it also shows your lack of fundamental programming skills. I’d love to see a case where you use throw/catch because there’s no other way. Only place I’ve ever used throw/catch is in my evil middleware Rack::Evil. And the name says it all.

Let’s take a real example from Rails :

def find_with_associations(options = {})
  catch :invalid_query do
    join_dependency = JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins])
    rows = select_all_rows(options, join_dependency)
    return join_dependency.instantiate(rows)
  end
  []
end

Just by looking at this method, you’ll have absolutely no idea who’s gonna be throwing :invalid_query. It could be any method subsequently called while the block is being executed. Only way to know is by doing a global search for throw :invalid_query.

Rails uses throw/catch here because it wants to return an empty array when something somewhere goes wrong. And the thing that can possibly go wrong is so deep down inside, throw/catch provides an easy way out without much refactoring. However, easy is not always the best way or the proper way.

If we look at the relevant code from the involved methods :

def select_all_rows(options, join_dependency)
  connection.select_all(
    construct_finder_sql_with_included_associations(options, join_dependency),
    "#{name} Load Including Associations"
  )
end

def construct_finder_sql_with_included_associations(options, join_dependency)
  scope = scope(:find)
  sql = "SELECT #{column_aliases(join_dependency)} FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} "

  ....
  if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
    add_limited_ids_condition!(sql, options, join_dependency)
  end
  ....

  sanitize_sql(sql)
end

def add_limited_ids_condition!(sql, options, join_dependency)
  unless (id_list = select_limited_ids_list(options, join_dependency)).empty?
    sql << "#{condition_word(sql)} #{connection.quote_table_name table_name}.#{primary_key} IN (#{id_list}) "
  else
    throw :invalid_query
  end
end

This doesn’t seem that bad on the first look. But think again. Apart from the control flow retardness, the method add_limited_ids_condition adds an extra responsibility to the caller – catching invalid_query. And this is very easy to miss too – as seen with the very same method in question here – calculations.rb. Add a few of more throw/catch and you get a proper spaghetti code.

I think the better way to write the above code is :

def find_with_associations(options = {})
  join_dependency = JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins])
  rows = select_all_rows(options, join_dependency)
  rows ? join_dependency.instantiate(rows) : []
end

def select_all_rows(options, join_dependency)
  finder_sql = construct_finder_sql_with_included_associations(options, join_dependency)
  connection.select_all(finder_sql, "#{name} Load Including Associations") if finder_sql
end

def construct_finder_sql_with_included_associations(options, join_dependency)
  ....
  limitable = !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])

  unless limitable && add_limited_ids_condition!(sql, options, join_dependency).blank?
    ....
    sanitize_sql(sql)
  end
end

def add_limited_ids_condition!(sql, options, join_dependency)
  id_list = select_limited_ids_list(options, join_dependency)
  sql << "#{condition_word(sql)} #{connection.quote_table_name table_name}.#{primary_key} IN (#{id_list}) " if id_list.present?
end

I’d normally say that you should be flexible about following such rules about using a pattern or not using some. But this is an exception. Using throw/catch is just fucking wrong. Plain and simple.