For something which indicates programmer error, such as the wrong type of argument passed to a method, definitely throw an exception. The exception will crash the program, drawing the programmer's attention to the fact that they are using your class incorrectly, so they can fix the problem. In this case, returning an error code wouldn't make sense, because the program will have to include code to check the return value, but after the program is debugged, such errors shouldn't ever happen.
In your WebCrawler
class, is it expected that crawl
will receive a bad URL as an argument sometimes? I think the answer is probably no. So raising an exception would be appropriate when a bad URL is passed.
When an exception is raised, the flow of execution suddenly "jumps" to the innermost handler. This can be a useful way to structure code when the exception is not expected to happen most of the time, because you can write the "main flow" of your method as simple, straight-line code without including a lot of details about what will happen when some rare error condition occurs. Those details can be separated from the "main flow" code, and put in an exception handler. When an error condition is expected to happen under normal conditions, though, it can be better to put the error handling code inline with the "main flow", to make it clearer what is going on. If the control flow of your program "jumps around" (as is the case when exceptions are used for normal flow control), that means the reader also has to "jump around" in the program text as they are figuring out how it works.
For the other two, I think it is expected that at least sometimes, the HTTP request will return an error code. To determine whether an exception or special return value is the best way to indicate such a condition, I would think about how often those conditions are going to happen under normal usage. Think also about how the client code will read either way. If you use exceptions, they will have to write something like:
urls.map do |url|
begin
crawl(url)
rescue PageNotFoundError
""
rescue UnauthorizedError
""
end
end
(By the way, I think this code example shows something: it might be a good idea if both of your custom exceptions inherit from a common superclass, so you can catch both of them with a single rescue
clause if desired.) Or if you use error codes, it would look something like:
urls.map do |url|
response = crawl(url)
if [:page_not_found, :unauthorized].include? response
""
else
response
end
end
Which do you think reads better? It's really up to you. The one thing which you do not want to do is use integer constants for errors. Why use integers? When you print them in a debug trace, you'll have to go look at the list of constants to see what each one means. And using symbols is just as efficient computationally.