New rule: single_case_function (or something better!)

[paulo-ferraz-oliveira]

If the only thing your function does is implement a case, move it to pattern matching (?)

[elbrujohalcon]

It's not that simple. The case should no be evaluating a function in its expression, nor computing a value that will be hard/impossible to compute in guards.

[paulo-ferraz-oliveira]

We can start simple. If it's a var or an atom, for example. But agreed, hard...

[elbrujohalcon]

Yeah, but I remember this rule and it's kind of the spirit behind this one.
This was never coded into Elvis, because it's really really hard to specify correctly.
Check this out…
Let's say we want to find places where we want to emit the warning.
We want a warning when Elvis finds…

A function clause that is composed of just one statement: a case statement.

• But what about this one?

function_clause() ->
    case rand:uniform(10) of
        X when X >= 5 -> top;
        _ -> bottom
    end.

• Let's refine it (as I mentioned before)…

A function clause that is composed of just one statement: a case statement with just function parameters as its expression.

%% This will catch this one...
function_clause(X) ->
    case X of
        X when X >= 5 -> top;
        _ -> bottom
    end.

%% But it won't catch this one...
function_clause(#{random_value := X}) ->
    case X of
        X when X >= 5 -> top;
        _ -> bottom
    end.

• Let's refine it…

A function clause that is composed of just one statement: a case statement with just a variable as its expression.
The variable can only come from the function clause arguments, because there are no other statements in the function.

%% This will catch this one...
function_clause(#{random_value := X}) ->
    case X of
        X when X >= 5 -> top;
        _ -> bottom
    end.

%% But it won't catch this one...
function_clause(X, Y) ->
    case {X, Y} of
        {X, Y} when X >= 5 -> Y;
        _ -> error
    end.

• Let's refine it…

A function clause that is composed of just one statement: a case statement with an expression built without using any function calls.

%% This will catch this one...
function_clause(X, Y) ->
    case {X, Y} of
        {X, Y} when X >= 5 -> Y;
        _ -> error
    end.

%% And also this one…
function_clause(X, Y) ->
    case [X, Y] of
        [X, Y] when X >= 5 -> Y;
        _ -> error
    end.

%% And this one…
function_clause(X) ->
    case #{random_value => X} of
        #{random_value := X} = Y when X >= 5 -> top;
        _ -> bottom
    end.

%% But not this one...
function_clause(X) ->
    case maps:from_list([{random_value, X}]) of
        #{random_value := X} = Y when X >= 5 -> top;
        _ -> bottom
    end.

%% And, depending on how it's implemented, it may or may not catch this one…
function_clause(X, Y, Z) ->
    case (X + Y) div Z of
        W when W >= 5 -> W;
        _ -> error
    end.

• To be honest… I'm not sure if I want a warning on the last one… consider the alternative…

function_clause(X, Y, Z) when (X + Y) div Z >= 5 -> (X + Y) div Z;
function_clause(_, _, _) -> error.

• Is it really better?
• And so far we've only talked about functions with a single clause… What if we have…

vertices(#shape{type = T}) ->
    case T of
        triangle -> 3;
        square -> 4;
        …
    end;
vertices(#polygon{type = T}) ->
    …
    .

• In this case, I'm not sure that using a different clause for each shape type will be any improvement. It's clear that the function semantically has a clause for each record it can receive… then it does what it needs to do there. It can be implemented using auxiliary functions (e.g., shape_vertices(T), etc.) but… is it worth warning about?
• On the other side… What about…?

my_function(#something_special{}) -> special:result();
my_function(X) ->
    case X of
        …

That does look like a fairly good place for a warning… maybe?

And we can keep writing examples and counterexamples for centuries to come…

In a nutshell: Yes, we can start simple… but it's worth noting (in the docs, at the very least… in the tests, hopefully) the rule limitations that we decide to include/oversee on its first implementation. :)

[paulo-ferraz-oliveira]

I'm Ok to postpone too. Glad we could discuss it. I'll close for now, and it becomes a historical reference, for future-us, future-consumer.