Ruby: Exceptions for flow control inside an .each block

Question

Consider the following minimal example:

class SomeException < StandardError
end

class Example
  @@logger = Logger.new
  @@failure_count = 0

  def do_a_thing(array)
    raise SomeException unless array.some_check?
    # more random code
  end

  def some_long_process(array)
    array.each do |item|
      begin
        do_a_thing(item)
        do_something_else(item)
        do_yet_another_thing(item)
      rescue SomeException
        @@logger.error 'Process failed'
        @@failure_count += 1
        next
       end
     end
   @@logger.info("Process done, #{@@failure_count} failures")
   end

The reason I've structured the code in this way is that this class's main job is to iterate through an array of hashes. If the do_a_thing task fails, I need it to log that failure, do some cleanup, and then skip the remainder of the steps in that begin block. Every possible failure in those steps is signaled by raising SomeException.

However, I'm aware that using exceptions for flow control is generally discouraged. My reasons for doing it here are:

1. The library (Mechanize) used in the various do_thing steps will automatically throw an exception if it gets a bogus HTML return back, which lends itself well to the logical organization of this class.

2. I want to count the errors on stdout, as this is part of a gem that will be used to do automated work.

3. I want to keep the logging for step failures in those methods if possible due to the large variety of possible failures, and use the external .each loop only for bookkeeping.

I want to know if there is a clearer/more idiomatic/fewer lines of code way to accomplish this same task. Is this the right way to be using exceptions?

Answer 1

This is not an example of using exceptions as flow control.

If you need this long running process to keep running, you must handle exceptions. You aren't changing the processing logic based on the exception being thrown. You are logging its occurrence and then proceeding to the next item in the array. The code is easy to read and understand.

This is just plain old "exception handling" and there is nothing wrong with it.

Answer 2

I think we can all agree that using Exceptions for flow control is an anti-pattern. (See @gnat's comment.)

Here's an alternative: Observers and Commands...

class Example
  attr_reader :logger

  def initialize(logger = nil)
    @logger = logger || Logger.new
  end

  def some_long_process(array)
    array.each do |item|
      next unless DoSomethingCommand.new(item, observers).execute
      next unless DoSomethingElseCommand.new(item, observers).execute
      DoYetAnotherThingCommand.new(item, observers).execute
    end

    Logger.info("Process done, #{aggregatingObserver.failed} failures")
  end

  private

  def loggingObserver
    @loggingObserver ||= LoggingObserver.new(logger)
  end

  def aggregatingObserver
    @aggregatingObserver ||= AggregatingObserver.new
  end

  def observers
    [loggingObserver, aggregatingObserver]
  end
end

class DoSomethingCommand
  attr_reader :item, :observers

  def initialize(item, observers)
    @item = item
    @observers = observers
  end

  def execute
    success = the_result_of_some_boolean_operation(item)

    if success
      observers.each { |observer| observer.success }
    else
      observers.each { |observer| observer.failure }
    end

    success
  end
end

class LogObserver
  attr_reader :logger

  def initialize(logger = nil)
    @logger = logger || Logger.new
  end

  def success
    # maybe you log success, maybe you don't
  end

  def failure
    logger.error('Process failed')
  end
end

class AggregatingObserver
  attr_reader :succeeded, :failed

  def initialize
    @succeeded = 0
    @failed = 0
  end

  def success
    @succeeded += 1
  end

  def failure
    @failed += 1
  end
end

Example.new.some_long_process([0, 1, 1, 2, 3, 5, 8, 13, 21])