Saturday, June 15, 2013

What's dirtier than comments? Exceptions!

I postulate that comments are a code smell.

Craig Buchek suggests a possible counterexample:
# Let this raise its exception if the fields don't exist as expected.
user = c.get_setting('username')
How else could one express the intention here, he asks?

I say, the output of a function should be expressed in its return value. Exceptions are cheating -- they bust out of normal flow control. That's a surprise, and surprises are to be avoided. No surprises, no comments needed.

Why is it throwing an exception? get_setting must be mucking around in the environment. The fetching of a setting from outside a program is a side effect, a dependency on the world around program execution, a bummer. It's necessary, so let's do this carefully.

Instead of throwing an exception, get_setting could return the failure information to its caller. This is a classic use for Either. In Scala:

def get_setting(settingName: String): Either[Error, String]

This method signature reports that you will get back the setting you want OR information about the error. (The convention is that Either contains the desired value on the right or the failure information on the left. In the type parameter list, that puts the failure type first, which is unfortunate.)
Then what does the calling function do with that Either it gets back? We don't like a whole bunch of "if it worked, do this" statements; that's not much cleaner than try/catching. Let's fill in some hypothetical code around Craig's snippet.

def getAuthorizationLevel(c: SettingGetter, 
                 authorizer: Authorizer): AuthLevel = {
    # Let this raise its exception if the fields don't exist as expected.
    user = c.get_setting('username')
    authorizer.getAuthorizationLevelForUser(user)
}

Here, the authorizer method can also throw an exception. If we replace exception-throwing with explicit error handling using Either, we get:

def getAuthorizationLevel(c: SettingGetter, 
                 authorizer: Authorizer): Either[ErrorAuthLevel] = {
    val possibleUser = c.get_setting('username')
    val possibleAuthpossibleUser.right.map{ user => 
      authorizer.getAuthorizationLevelForUser(user) }
    possibleAuth.joinRight
}

OK. This is cryptic if you aren't used to working with Either. (I had to look up joinRight.) What's happening?

right.map says, "If we have a string, replace it with the output of this function." We take that string (user) and turn it into... what? Here's the signature for the next method call:

def getAuthorizationLevelForUser(user: String) : Either[Error, AuthLevel]

This output will replace the user string. We took possibleUser, which was either an error or a string, and now in possibleAuth we have either an error (from user) or an error (from auth) or an AuthLevel. It's Either[Error, Either[Error, AuthLevel]]. Yuck!

joinRight says "If we have an Either on the right, return that instead of this one." We get an Either[ErrorAuthLevel]; the Error could be from getting the user setting or from getting the authorization level. joinRight scrunched the two errors into the same place.

That feels like a lot of work to avoid throwing an exception. Yet, the comment isn't necessary because the possibility of failure is explicit in the return values. There's no if/else or try/catch blocks; instead, processing based on the user setting is moved into the context of the Either, so that we fetch the auth level only if we got that user string.

Now the possibility of failure is returned explicitly to the caller of getAuthorizationLevel. If they want to report an error back up the chain, great. If they log the error and then use a default authorization level, even better. In case you're curious, that would look like this:

val possibleAuth = getAuthLevel(settingGetter, authorizer)
possibleAuth.left.foreach(error => log(error))
val authLevel: AuthLevel = possibleAuth.right.getOrElse(AuthLevel.GUEST)

Errors are data, too. Return them to your caller in the same flow of control as the data you were hoping to supply.

-----------------------------------
Scala code for this post: https://gist.github.com/jessitron/5788968

@marioaquino agrees with me: http://marioaquino.blogspot.com/2013/06/styles-of-handling-conditional-logic.html

So does @heathborders: http://heath-tech.blogspot.com/2013/06/all-smart-kids-are-writing-blog-posts.html

and @Adkron, although he loves OO more: http://dirtyinformation.com/blog/2013/06/15/exceptional-comments/

... now I hope @CraigBuchek will chime in with an opposing point of view because we're running out of argument here.

and here's a timely one from @josefusbarnabus that agrees comments are evil: http://heath-tech.blogspot.com/2013/06/all-smart-kids-are-writing-blog-posts.html

and alonzophoenix says types are better than comments: "type annotations—like pretty much everything else in programming language source code—are best thought of as more for human consumption than for the compiler." http://archontophoenix.blogspot.com/2013/04/no-comment.html

34 comments:

  1. mmmm.... I might buy this, but I really need to think about it.

    There are lots of kinds of exceptional conditions. One that springs to mind is Java's OOM. While the semantics of Java imply that you intercept it, you really can't. I'd be delighted to hear that Scala had a way to handle this condition -- one that was better than Exception -- but it surely isn't an Either.

    -blake

    ReplyDelete
    Replies
    1. I agree completely about OOM.

      Error conditions like that -- all the Errors in Java (the ones that are Throwables but not Exceptions), really should propogate upward. Those aren't data. Your program can't continue to run. They're a good reason to break normal flow.

      (It bugged me to call the error type Error above, but anything longer didn't fit well. In real life I'd use something more specific.)

      Delete
  2. I've been adopting this approach of using Either in my open source project (http://snmp4s.org). I still have some thinking to do to keep it from being a mess of a nested list of Eithers. Thanks for the tips on how to collapse them together! That gives me some good ideas to clean it up.

    I also agree with Blake, tho. There are some errors that you simply can't handle. (Unchecked) exceptions make it nice because you don't have to clutter up the code with errors that will be propagated up. However, I think the gain from having the errors explicitly in your return type outweighs the inconvenience.

    ReplyDelete
  3. Yes. While the title implies that this is about Exceptions, (which is, btw, the word commonly used to discuss behaviors implemented in Java using the class Throwable), it is, in fact, about a much smaller case. Based on a short twitter exchange, it is not intended to address either Java Errors, or RuntimeExceptions thrown by other components.

    Given that, this post seems to be about the syntax used to handle corner cases. Fair enough. Except for the joinRight magic, the code in it looks very nice.

    Within, at least, the Java community, the difficult discussion has always been making the judgement call about whether an exceptional condition is something that should be visible to the caller or not: when something should be a RuntimeException. Some frameworks have well-developed Exception management strategies (including wrapping them at API boundaries). Others just make everything un-checked.

    ReplyDelete
  4. Depends. If 'username' was something obtained from and user interface and c was an instance of a type not in the user interface layer, eventually you'd have to communicate the failure up the stack to something that could inform the user of the problem. (this could be simply data validation, or something more). If you *didn't* use exceptions you'd have to write code in each function all the way up the stack to where something used it. Everything in between would really then be more coupled to what calls it. Not to say this isn't useful in certain circumstances; but, when you've got 20 or so levels of method calls, this can be come tedious and error-prone. In the case of 'username', I don't think it's a surprise to expect that it was validated at a higher level and if get_setting can't succeed, that it would be exceptional and throw an exception. i.e. I'd never want to call get_setting without pre-validating the data I give it at a much higher level.

    ReplyDelete
    Replies
    1. I imagined that 'username' is the name of the setting, like it's pulling that out of the OS somehow. So not user input. 'Cuz you're right, validating user input should be a separate step, and then fetching it should not fail in a recoverable way.

      Still, passing data up twenty method calls isn't as bad when everything uses right.map (or mapRight in other languages), performing processing in the context of the exception. It's explicitly handling errors without any if/then or try/catch logic at each level.

      Delete
  5. NO NO NO NO NO, GOD NO!!

    This is the paradigm we had in c and it really really limits how many levels of depth you can have between where the exception occurs and where it is caught. for example if
    a calls b calls c calls d calls e calls f calls g calls h call parseInt then if that is going to be caugtht at the top abcdefgh all need to check and pass the arguments.

    This is horrible, creates a lot of icky code, and tends to have exceptions just get lost in the 1 method that gets lazy and forgets to check.

    Exceptions should break flow. The example you are giving is more of a validation that an exception which is why you are wanting to handle it immediately, but most places where exception are thrown do not want to be handled immediately.

    ReplyDelete
    Replies
    1. Well, like Peter said, should you really wait so long to validate input? go that deep?

      Bad user input is not an unexpected condition. Exceptions should be reserved for unexpected conditions.

      What I like about Either (vs null checks or try/catch) is that you're still passing exactly as many arguments. And you're not so much checking them every time as executing code within the context of "data that we may not really have."

      C couldn't use this method, because it didn't have clean, readable first-class functions. (No, function pointers don't count.) Now that we have those, it's time to revisit passing errors back up as data. You couldn't mapRight on two separate arguments in C.

      Delete
    2. Funny! My initial reaction, on reading this post, was much like Llewellyn's. On consideration, though, I realized that, actually, it is C and Java that have styles that are the opposite extremes in handling edge cases. In Java, you return the functional value and throw exceptions to handle the edge cases. In C, functions return status and use inout parameters to return values. While there is still the uncomfortable discussion about what is "unexpected" and what isn't, I think Jessica's point, about completely different return mechanisms for data that is actually quite similar, is very well taken

      Delete
    3. Also, the _huge_ difference between C and Scala for this sort of thing is that Scala has useful syntax to make this much easier. When you put this sort of thing in nested for-comprehensions, you don't have to do all that manual checking.

      Delete
  6. Interesting post Jessica. I'm not a Scala programmer, but I'd argue the either pattern risks failing slow. An exception must be explicitly handled by the caller or it will bubble up. With the either pattern (at least in C# where I live) the caller may accidentally ignore the either and thereby fail slow, producing unpredictable results that are tricky to debug. Am I missing something?

    ReplyDelete
    Replies
    1. It's hard to ignore the error when the type system forces you to root around in an Either to get the (possible) answer. At least, it's explicit in the return type that it might contain an error, unlike a maybe-might-be-null reference.

      When subsequent code is inside a mapRight, it isn't failing any more slowly than an exception. Throwing an exception halts the method before the rest executes; mapRight skips the rest of the method. In both methods the error bubbles up. Either is more explicit than a runtime exception and more flexible than a checked exception.

      Delete
    2. You typically put these in a for-comprehension:

      val result = for {
      a <- doThingAThatReturnsAnEither
      b <- doThingB....
      c <- doTh....
      }

      That way it's checked for you; the type of a/b/c are the right side of the Either, and if any of them return a left-value the result of the entire comprehension is that Either.

      Well, that's true if you're using Scalaz's \/ - the types are something like:

      val a: String
      def doThingAThatReturnsAnEither: Int \/ String

      where the failure type is on the left and the success type is on the right. (I haven't used the built-in either for a while; I think you have to jump through more hoops to make it right-biased)

      Delete

  7. I'm surprised no one has mentioned the m-word yet. You're effectively talking about plumbing here, and plumbing is one use case for the m-things.

    What's the code you'd like to write? I think it is this.

    getAuthorizationLevel c authorizer
    = do
    possibleUser <- c.get_setting('username')
    authorizer.getAuthorizationLevelForUser(user)

    We can use the monadic pattern here, of composing operations in the presence of lumps (here, the lumps are "operations which might fail"), ie. do the first lump, and pass the result to the second lump. The definition of the monadic pattern for Either basically provides the plumbing you want, namely stop at the first failure else keep going - and it's defined like this:

    Left e >>= _k = Left e -- stop and return error x
    Right x >>= k = k x -- propagate x and continue

    Some people describe monads as programmable semicolons, and that's indeed what is happening here. We're kind of replacing the baked-in exception mechanism with something we have more control over, without losing too much convenience but it's much more up front and controllable. There's other advantages too, like we can easily add info to the error message by (functorial) map over the error structure before it goes back to the caller.


    But yes: it's a good plan to move vague details in comments and exceptions into the code via the type system, so that the compiler can make more use of them and check that the code makes sense. (And if doing it in types isn't an option yet, it's also good to convert the comments into tests - you can still get some value from them that way.)

    ReplyDelete
    Replies
    1. Shh! Don't tell them it's a monad! Then they'll think it's all mathy. Let Either be a class, and then it looks object-y.

      So far my conception of monads is: context. In this case, run the second lump (nice word) in the context of the first lump's success.

      There's a balance in abstraction: too much terseness leads to obfuscation. Starting out, it's better to see more of the plumbing. Once you're comfortable with how it works, then hiding it only helps.

      Delete
    2. "starting out..." - maybe, but it's important to let them know that the destination is worth the journey!

      Delete
  8. Since most classes aren't as much types as mere namespaces, why not give all domain data it's own type, not just the aggregates. the venus factor does it work

    ReplyDelete
  9. What about when mapping futures? Would you still use Either or Try for the mapping functions?

    ReplyDelete
  10. Thanks to an wide variety of mobile cellphone programs and thinking processing, organizations can often find out out most of the facts they need, and handle their day, without ever discussing with the house organization office. buy joey atlas naked beauty symulast method

    ReplyDelete
  11. Chicago, illinois, il, about the very subject of organization events. Indicate has a success of experience with preparing organization events and he has hand to create AHR the effective display that it is nowadays. bitcoin hosting

    ReplyDelete
  12. Boulder Electric Vehicle and a Precision customer, invented the electric service truck, and convinced Robichaud to test-drive the truck that he felt would be a perfect fit for the service industry because of the short routes service technicians drive daily. f4x exercise program

    ReplyDelete
  13. I've already started using some of them. There wasnt a major growth but my boobs were a little more plump and full. Disappointed that it did not include a miter guide. I have even made adjustments so I can throw in a little ground flavored coffee while still using the grinder. f4x workout reviews

    ReplyDelete
  14. Both arrived the financial system in the dump and discredited their specific events. look at here now

    ReplyDelete
  15. Unchecked) exceptions make it nice because you don't have to clutter up the code with errors that will be propagated up. However, I think the gain from having the errors explicitly in your return type outweighs the inconvenience. next story

    ReplyDelete
  16. I also used a chain of them to hook up toys to the stroller bars, so he could freely play with them but they won't drop to the floor. Just remember to buy spare filters, light bulbs and the powercable. www.browneshealth.co.uk

    ReplyDelete
  17. Thanks to an wide variety of mobile cellphone programs and thinking processing, organizations can often find out out most of the facts they need, and handle their day, without ever discussing with the house organization office. address

    ReplyDelete
  18. This sounds like exactly the kind of testing I'd like to see side-by-side with the TDD cycle. It sounds like an ideal way for programmer and tester to work together in real time on the same task. It also sounds like a great technique for programmers to practise in order to ingrain a few extra useful testing heuristics, which might reduce their blindspots and improve their effectiveness when test-driving anything. leptin

    ReplyDelete
  19. It is our intention to drive the first Electric Van for approximately 90 days and discover all of the nuances that might come with an EV or if there are any design changes we would like to have included with future vans,” said Robichaud. Saran Wrap Weight Loss

    ReplyDelete
  20. You can price the home to earn profits. However, if you want to create a fast selling of your home, price to crack even. Sell My Home

    ReplyDelete
  21. This allowed a large number of devoted football followers to obtain simple and easy access to bet on their favorite soccer groups or players. Taruhan Bola

    ReplyDelete
  22. Also choosing the incorrect shade colour or material may well be a beloved mistake that you obviously should identify for an extended time or utilize significant amounts of cash to negotiate. click to find out more

    ReplyDelete
  23. The individual produces uncommonly larger facial bone fragments, changing the overall overall look from the face. The musculature deteriorates as well as the person has a higher tendency of creating heart and diabetic person problems, such as hypertension and lack of feeling in areas where side-line anxiety are present. important link

    ReplyDelete
  24. One neat thing about being the first company to purchase the trucks is that Robichaud is pretty much involved in any design changes that may be made to the vehicle. indoor hobbies for men

    ReplyDelete
  25. Clean toys and games in hot, standard soapy water, wash and dry. Colorfast toys and games can be saturated in a lighten solution of cup of household lighten in 1 quart of h2o. click

    ReplyDelete