Avoid Using Exceptions for Flow Control
Avoid Using Exceptions for Flow Control
See an alternative way to using exceptions, which can harm performance and cause dependency problems, when dictating your flow.
Join the DZone community and get the full member experience.
Join For FreeThis code is taken from discourse code base, but similar code can be found in most of the applications.
def execute(args)
raise Import::ImportInProgressError if Import.is_import_running?
raise Export::ExportInProgressError if Export.is_export_running?
@format = args[:format] || :json
# additional initialization here
start_export # start_export definition is below
# business logic
end
def start_export
if @format == :json
@encoder = Export::JsonEncoder.new
else
raise Export::FormatInvalidError
end
# ...
end
There is a lot of code in this file (most was removed for clarity), but in general, the flow comes down to:
Checking the precondition (and fail with one of
Import::ImportInProgressError
,Export::ExportInProgressError
, andExport::FormatInvalidError
errors in case you can't start).Processing the export.
Clearing the environment after the export.
At first glance, it may seem that everything is OK, but it's not. Some would say because "exceptions are expensive" (in terms of performance), which is true but this is not why. In general, when analyzing a process that you didn't write, usually the flow is:
Browse the code and read the documentation, if any. Usually, there is no documentation, so you end up browsing the code (or ask a colleague if possible).
Check what the input data are.
Check what the output data are.
This code actually breaks the third point. If you had a client with this code and you wanted to cover failing scenarios, you would have to write something like this:
def client_that_runs_execute
data = prepare_data
result = process.execute(data)
process_result(result)
rescue Import::ImportInProgressError, Export::ExportInProgressError, Export::FormatInvalidError => e
handle_export_not_processed_error(e)
rescue => e
handle_unexpected_export_error(e)
end
So, you no longer have one kind of output. It's either the process result or errors.
Whether it's a problem or not depends on what is the logic behind the handle_export_not_processed_error
. The more complex error handler, the more difficult code will be to maintain. If you write tests, you force yourself to mock `execute` and raise an exception. Then you have to check whether appropriate steps were taken. The business logic has a tendency to become more and more complex. Approaching with errors will make it harder to develop it.
Additionally, if you have more than one client using this code, each has to implement separate rescue methods (tests irritate even more).
Lastly, exceptions describe the truly exceptional situation. Does ExportInProgressError
really represent that?
Solution
OK, but what can we do instead? In our example, all the errors are thrown before the processing begins, in precondition checking step. I would suggest creating separate validator which decides whether we may start processing or not. If we can't (preconditions failed) then the whole process stops and exits with an error code/message. An example below:
module ExportValidator
SUPPORTED_FORMATS = Set.new([:json])
def self.can_start?(format)
errors = []
errors << "another import is running" if Import.is_import_running?
errors << "another export is running" if Export.is_export_running?
unless SUPPORTED_FORMATS.include?(format)
errors << "unsupported format: #{format}"
end
errors
end
end
def execute(args)
@format = args[:format] || :json
errors = ExportValidator.can_start?(@format)
return error_result(errors) unless errors.empty?
start_export
end
def error_result(errors)
res = ExportResult.new
res.success = false
res.errors = errors
res
end
Not Dogma
Can we make any exception to don't-control-flow-with-exceptions rule? Of course we can. This is not dogma, but I personally try not to overuse them. In my opinion, the deeper the layer (service, persistence) the fewer number of exceptions should be used because it's harder (thus more error prone, and more expensive in terms of time [money]) to change the code because the layers above already depends on this behavior, and you may not be able to detect all the scenarios that require changes (regression should be expected). On the other hand, if the application is relatively small, refactoring it may not be a problem.
Published at DZone with permission of Damian Jaszczurowski . See the original article here.
Opinions expressed by DZone contributors are their own.
{{ parent.title || parent.header.title}}
{{ parent.tldr }}
{{ parent.linkDescription }}
{{ parent.urlSource.name }}