views:

248

answers:

4

Say I have some function fn1() in Erlang which returns {ok, Result} if the function was executed successfully and {error, "ErrorReason"} if there was an error.

Now in another function fn2() I call fn1() and I need to check the result of fn1 and proceed only if it is {ok, Result}.

I figured, I can do this using either case or try catch. But Efficiency is my main concern and I'd like to know which of the two methods below is more efficient:

try-catch Method

fn2() ->
   try
      {ok, Result} = fn1(),
      %Do something with Result
      ok
   catch
      throw:Term -> Term;
      exit:Reason -> {exit, Reason};
      error:Reason -> {error,{Reason,erlang:get_stacktrace()}}
   end.

case Method

fn2() ->
   Res = fn1(),
   case Res of
      {ok, Result} -> 
         %Do something with Result
         ok;
      {error, Reason} ->
         Reason
   end.
+6  A: 

The case method will be more efficient, as it simply pattern matches, and does not involve building a call stack and stuff.

In both examples you are about to handle the "error" locally, so there is no point in the try catch.What you might see sometimes is something like:

fn2() ->
  {ok, Result} = fn1(),
  %Do stuff with Result
  ok.

Here the intention is that you make fn2() throw a badmatch, if fn1() did not return ok. You let someone else "above" handle the problem. E.g. this might kill your process, and make your supervisor create a new one.

Zed
Thanks, that was enlightening! :)
ErJab
+2  A: 

You should always measure to find things like this out.

Your code also does not do what you think it does.

-module(glurk).
-compile(export_all).

fn2() ->
   try
      {ok, Result} = fn1(),
      %Do something with Result
      ok
   catch
      throw:Term -> Term;
      exit:Reason -> {exit, Reason};
      error:Reason -> {error,{Reason,erlang:get_stacktrace()}}
   end.

fn1() ->
    {error, a}.

Try this out:

 c(glurk).   
./glurk.erl:6: Warning: variable 'Result' is unused
{ok,glurk}
16> glurk:fn2().
{error,{{badmatch,{error,a}},
        [{glurk,fn2,0},
         {erl_eval,do_apply,5},
         {shell,exprs,6},
         {shell,eval_exprs,6},
         {shell,eval_loop,3}]}}

This is because fn1 did not raise an exception it gebnerated a normal retyurn value {error, a} which does not pattern match against {ok, Result}

The first version of your code works with a function that either returns a normal value or raises an exception - you have to write it like this:

fn1(....) ->
     ...
     %% success case
     Val;

     %% failure case
     throw(...) | exit(...) | error(...)

You can't just pump the same function into fn1 and fn2.

If you had the case where the called function had to escape from a deep recursion then the first method would be more efficient than the second - since you could immediately exit from a deep recursion by saying throw(...).

So the answer depends upon the nature of the function that you are calling.

Code should always be optimised for beauty and not efficiency - since you have to maintain the stuff - then it should only be optimised in the rare cases where it is not fast enough. What needs to be optimised should be identified by measuring the program (you will always be surprised here :-)

Me, I'd write

{ok,Result} = ...

Actually your first code has a more subtle error

 fn2() ->
   try
      {ok, Result} = fn1(),
      %Do something with Result
      ok
   catch
      throw:Term -> Term;
      exit:Reason -> {exit, Reason};
      error:Reason -> {error,{Reason,erlang:get_stacktrace()}}
   end.

Think about this. The caught error cases do not themselves handle the error they just return tuples like {exit, Reason} or {error, Reason} this means that the next layer up (ie the caller of fn2) will also have to mess around checking error returns - if this is repeated at all levels the code will be a mess.

The "erlang" way is to have one try-catch at the top of the program and just terminate abruptly with exit(Why) if an error occurs.

In fact often you should not even do this - you should link your process to another process then the offending process will die and "the other processes will fix the error".

The exception propagates up the call stack and flies over to the linked processes for treatment. So we have two types of processes - ones that have no inbuilt error handling and processes that only do error handling.

ja
Yes, always benchmark.
Christian
+4  A: 

You really want to try and avoid try/catch like the plague. It is a very uncommon idiom in Erlang - really only used in a couple of special cases:

  • where you are checking user-supplied input and you have no guarantees that it will be 'correct'
  • where you have something which is deeply nested and the cost of unrolling it on an error condition is too expensive
    • like mensia transactions
    • or in parser/lexer's

Try/catch is essential in languages like C++ where the application is unstable in the presence or errors, but Erlang is stable in those circumstances - the process crashes but doens't bring the system down.

You should programme the happy path, match return values and if the application deviates from what you expect then let it crash. The crash tells you you have a problem and tells you to fix it.

The problem with try/catch is that it can simply mask the problem, or even worse, move the eventual crash away from where it should happen (inside the expression you have wrapped) and make it appear elsewhere - where your programming logic expects it to have suceeded = which makes debugging much harder.

Programming the happy path and letting it crash is very disconcerting the first time you do it, it feels like going out with no clothes on, but actually you get used to it real quick :)

Gordon Guthrie
+1  A: 

In this case, irrespective of what is more efficient, you should definitely use the case alternative as it more succinctly describes what is going on. Your fn1() here return a value indicating if there is a successful value or an error. In the case version you directly match against this, while in the try version you match against the success value which will generate an error if an error was returned. It is unnecessarily convoluted and hides what is going on so it is bad programming style and should be avoided.

And as Gordon has already pointed out having a try there will catch more errors than you probably intend and may so mask other real errors which you should see.

The case will also be faster here, but the difference is probably small. The clarity, succinctness and good programming style is much more important!

rvirding