Newbie code smell: explicit function clause error | Elixir Language | Page 1

white cipher Jan 5, 2024, 10:22 AM

#

In Elixir, to my understanding, explicitly writing raise(FunctionClauseError) is a huge code smell.

But is there any better way to write the following?

@spec calc(input :: pos_integer()) :: non_neg_integer()
# a base case
def calc(1), do: 0

# can't accept non-numbers and non-positive inputs
def calc(input) when not is_number(input), do: raise(FunctionClauseError)
def calc(input) when input <= 0, do: raise(FunctionClauseError)

# my computations, we can conceptually ignore what I'm doing here below
def calc(input) when (input &&& input - 1) == 0, do: :math.log2(input)
def calc(input) when Kernel.rem(input, 2) == 0, do: 1 + calc(Kernel.div(input, 2))
def calc(input), do: 1 + calc(3 * input + 1)

Without having to specify the first two conditions by hand, on each other clause, aka:

# here, I *must* write guards for each match, if I want to avoid Math errors!
def calc(input) when is_number(inpu) and input>0 and (input &&& input - 1) == 0, do: :math.log2(input)
def calc(input) when is_number(inpu) and input>0 and Kernel.rem(input, 2) == 0, do: 1 + calc(Kernel.div(input, 2))
def calc(input) when is_number(inpu) and input>0, do: 1 + calc(3 * input + 1)

To some extent I like raising FunctionClauseError because I clear the solution space beforehand, so I wonder what' the best call here.

queen river Jan 5, 2024, 10:28 AM

#

Not sure about better, but alternatives:

# Separate the implementation into a private function so you can define one interface with a common guard
@spec calc(input :: pos_integer()) :: non_neg_integer()
def calc(input)

# a base case
def calc(1), do: 0

# can't accept non-numbers and non-positive inputs
def calc(input) when is_number(input) and input > 0, do: do_calc(input)

defp do_calc(input) when (input &&& input - 1) == 0, do: :math.log2(input)
defp do_calc(input) when Kernel.rem(input, 2) == 0, do: 1 + calc(Kernel.div(input, 2))
defp do_calc(input), do: 1 + do_calc(3 * input + 1)


# Or use defguard to define a guard and then use it in your clauses:

def calc(input) when is_pos_integer(input) and (input &&& input - 1) == 0, do: :math.log2(input)
def calc(input) when is_pos_integer(input) and Kernel.rem(input, 2) == 0, do: 1 + calc(Kernel.div(input, 2))
def calc(input) when is_pos_integer(input), do: 1 + calc(3 * input + 1)

white cipher Jan 5, 2024, 10:30 AM

#

I like the "private" approach better. Thank you a ton, I'll try this ASAP

#

Yeah it works great! I like this approach, incapsulation rules, so I'll mark this as solved (:

queen river Jan 5, 2024, 10:38 AM

#

oh a third option as food for thought:

@spec calc(input :: pos_integer()) :: non_neg_integer()
def calc(input)

def calc(1), do: 0

def calc(input) when is_number(input) and input > 0 do
  cond do
    (input &&& input - 1) == 0 -> :math.log2(input)
    rem(input, 2) == 0 -> 1 + calc(div(input, 2))
    true -> 1 + calc(3 * input + 1)
  end
end

white cipher Jan 5, 2024, 1:01 PM

#

Nice! cond reveals itself very useful here. But I like the pattern matching clauses better

#Newbie code smell: explicit function clause error