Should I use 'if' or function overloads?

Questions / Help
1

Hi all,

I wonder which one should I go for. The if approach looks straightforward, but not sure which one is most recommended practice for Elixir.

  1. if
def handle_event("guess", %{"number" => guess}, socket) do
    current_score = socket.assigns.score
    secret_number = socket.assigns.secret_number
    {message, score} = check_guess(guess, current_score, secret_number)

    {
      :noreply,
      assign(
        socket,
        message: message,
        score: score
      )
    }
  end

  def check_guess(guess, current_score, secret_number) do
    {guess_number, _} = Integer.parse(guess)

    if guess_number == secret_number do
      message = "You've won!"
      score = current_score + 1
      {message, score}
    else
      message = "Your guess: #{guess}. Wrong. Guess again"
      score = current_score - 1
      {message, score}
    end
  end
  1. function overloads - seems more codes than if approach, and kinda of repetitive
def handle_event("guess", %{"number" => guess}, socket) do
    current_score = socket.assigns.score
    secret_number = socket.assigns.secret_number
    {message, score} = check_guess(guess, current_score, secret_number)

    {
      :noreply,
      assign(
        socket,
        message: message,
        score: score
      )
    }
  end

def check_guess(guess_number, current_score, secret_number) do
  {guess, _} = Integer.parse(guess_number)
  
  score = update_score(guess, current_score, secret_number)
  message = generate_message(guess, secret_number)
  
  {message, score}
end

defp update_score(guess, current_score, secret_number) when guess == secret_number, do: current_score + 1
defp update_score(_, current_score, _), do: current_score - 1

defp generate_message(guess, secret_number) when guess == secret_number, do: "You've won"
defp generate_message(guess, _), do: "Your guess: #{guess}. Wrong. Guess again."

Besides that, what if I have nested if to cover more complex scenarios, any recommendations to write better?

Thank you.

2

I would go for 1 because you’ve got three functions that are called once each in 2. You should parse the string before calling check_guess. In that scenario, you could do two functions heads for check_guess but it’s fine either way.

If this is the only place check_guess is called, I would remove it and put the if statement in handle_event.

When things get complex there are several tools: cond, case, multiple function heads. It all depends on the situation.

1 Like
3

The guards are redundant, and I’d rearrange the functions as:

defp get_result(n, n, current_score), do: {"You've won", current_score + 1}
defp get_result(guess, _, current_score), do: {"Your guess: #{guess}. Wrong. Guess again.", current_score - 1}
3 Likes
4

When you said guards are redundant, hmm… how do you compare the guess == secret_number ?

I tried your approach, and it flagged me - the 2nd function - this clause for get_result/3 cannot match because a previous clause always matches.

Any idea?

5

You didn’t do n, n.

1 Like
6

An explanation is that we are using pattern matching here instead of a guard. The first n matches the second n in the function head, both are bound to the same variable. Then we have the fallback function head, much like the one you have for the guards.

Some would argue that pattern matching like this is hard to read.

2 Likes
7

wow, didn’t know can do this. Anyway, I just watched a recursion tutorial and it uses the pattern matching, now makes sense. But ya, it’s hard to read though.

So I guess guard will make the code easier to read than this way?

8

It is a style choice. If you work in a code base with heavy use of pattern matching then you’ll get used to it. One benefit is that the lines are shorter, and the level of nesting will be lower. Code can be read in a straight line down the file.

1 Like
9

I’m the camp of “harder to read” but I do use it sometimes. I usually name it something descriptive like same_number when I do.

3 Likes
10

Yes, I would avoid using abbreviations when doing “same” pattern matches. I had a situation a few weeks back where I was so confused why this code wouldn’t work :wink:

  @maybe_datetime_without_seconds_regex ~r/(\d{4})-(\d{2})-(\d{2})\s(\d{1,2}):(\d{2})\s?(\w+)?/
  
  defp maybe_datetime_without_seconds(val) do
    captures = Regex.run(@maybe_datetime_without_seconds_regex, val, capture: :all_but_first)

    if captures do
      maybe_datetime = Enum.map(captures, fn capture ->
        case Integer.parse(capture) do
          :error -> capture
          {i, _rest} -> i
        end
      end)

      case maybe_datetime do
        [y, m, d, h, m, tz] ->
          DateTime.new(Date.new!(year, month, day), Time.new!(hour, minute, 0), tz)
        [y, m, d, h, m] ->
          NaiveDateTime.new(Date.new!(year, month, day), Time.new!(hour, minute, 0))
      end
    end
  end
3 Likes