[erlang-questions] case expression scope

[Richard A. O'Keefe]

On 5/03/2014, at 5:10 AM, Daniel Goertzen wrote:

> I've been reflecting on this some more, and have realized one of my biases on the subject comes from the way C/C++ manages scope:  Anything within curly braces, whether it is a function, if, while, for, try, or catch, is a scope which encapsulates all declarations within.  This uniform rule makes it easier to reason about code, and not having it in Erlang is, well, jarring to me.
> 
> Now both Erlang and C/C++ can *read* variables from enclosing scopes, but only C/C++ can *mutate* variables from enclosing scopes.  Perhaps Erlang's case scoping rules are just a way to give similar powers to affect the enclosing scope.

No, you are still thinking in C/C++ terms.
There are three  things in Erlang that are scopes:

	- a function clause is a scope
	- a 'fun' is a scope
	- a list comprehension is a scope

In something like

	f(X, Y) ->
	    case Y
	      of {A,B} -> Z = 1
	       ; [A,B] -> Z = 2
	    end,
	    {foo,A,B,Z}.

there is no "leakage" from one scope to another and
there is no "affect[ing] the enclosing scope" because
THERE IS ONLY ONE SCOPE.

> The actual clause that predicated all this is here:  https://gist.github.com/goertzenator/9347573

Yike!  I have never shared the vulgar prejudice against
long functions, but 78 lines for a single clause?

I would _definitely_ start by breaking out little functions.

engine_id(#{engine_id := local}) ->
    snmp_agent_controller:get_engine_id();
engine_id(#{engine_id := E})
  when is_list(E) ->
    E.

authkey(#{authkey := AuthKey}, _, _)
  when is_list(AuthKey) ->
    AuthKey;
authkey(#{authpassword := AuthPass}, Localization_Hash, Engine_Id) ->
    snmp:passwd2localized_key(Localization_Hash, AuthPass, Engine_Id).

privkey(#{privkey := PrivKey}, _, _)
  when is_list(PrivKey) ->
    PrivKey;
privkey(#{privpassword := PrivPass}, Localization_Hash, Engine_Id)
  when is_list(PrivPass) ->
    lists:sublist(snmp:passwd2localized_key(
        Localization_Hash, PrivPass, Engine_Id), 16).

...

> There are a lot of case expressions that leave unwanted bindings lying around.

There shouldn't _be_ a lot of case expressions in a single
function clause.  _That_ is the problem.

> I have to pay attention to use different binding names for each to avoid collisions.  Specifically the AuthKey and PrivKey expressions had collisions at first because they are nearly identical.  The code would be a lot easier to reason about if I didn't have to look out for such things.

And if you split them out into separate functions,
you _wouldn't_ have to look out for such things.
> 
> I tried putting this function together in various different ways, and this way was the most concise, most readable, and most amenable to future tweaking.

If this was the most readable, I really do not want to see the
other versions!  I find the code quite unreadable (the
RunTogtherVariableNames don't help) and splitting out little
functions made it MUCH easier for me to see what's going on.

As for "most amenable to future tweaking", I'd like to point
out an advantage of splitting things out as functions:

  ++ you can give them types and have the types checked. ++

> I see I could also use funs directly instead of cases.
> 
> f1(X, Y) ->
>     A = fun({0, Q}) -> Q;
>            ({P, Q}) -> P*Q
>            end({X,Y}),
> 
>     B = fun({0, Q}) -> Q;
>            ({P, Q}) -> P+Q
>         end({X,Y}),
>     {A,B}.

You do not need those tuples.

f1(X, Y) ->
    A = (fun (0, Q) -> Q
           ; (P, Q) -> P*Q
         end)(X, Y),
    B = (fun (0, Q) -> Q
           ; (P, Q) -> P+Q
         end)(X, Y).

I would write

      {AuthP, LocalizationHash} = case maps:get(authp, Record) of
                                                            md5 -> {usmHMACMD5AuthProtocol, md5};
                                                            sha -> {usmHMACSHAAuthProtocol, sha};
                                                            usmHMACMD5AuthProtocol -> {usmHMACMD5AuthProtocol, md5};
                                                            usmHMACSHAAuthProtocol -> {usmHMACSHAAuthProtocol, sha}
                                                        end,
 as

    Localization_Hash =
        case maps:get(authp, Record)
          of md5                        -> md5
           ; sha                        -> sha
           ; usm_HMAC_MD5_Auth_Protocol -> md5
           ; usm_HMAC_SHA_Auth_Protocol -> sha
        end,
    Auth_Protocol =
        case Localization_Hash
          of md5 -> usm_HMAC_MD5_Auth_Protocol
           ; sha -> usm_HMAC_SHA_Auth_Protocol
        end,