Control flow structures vs Pattern Matching

Chat / Discussions
,
1

Hello,

I came to make a provocation about a way of writing code.

I’m writing a small private project to shorten urls and I came across a change in the writing of a case.

I saw that when writing the case in my function I felt that the function seemed complex to understand and read. I thought about changing the case to use Pattern Matching of the functions.

leaving this code:

defp validate_url(changeset, field, opts \\ []) do
    validate_change(changeset, field, fn _, value ->
      case URI.parse(value) do
        %URI{scheme: nil} ->
          "is missing a scheme (e.g. https)"

        %URI{host: nil} ->
          "is missing a host"

        %URI{host: host} ->
          case :inet.gethostbyname(Kernel.to_charlist(host)) do
            {:ok, _} -> nil
            {:error, _} -> "invalid host"
          end
      end
      |> case do
        error when is_binary(error) -> [{field, Keyword.get(opts, :message, error)}]
        _ -> []
      end
    end)
  end

for this:

  defp validate_url(changeset, field, opts \\ []) do
    validate_change(changeset, field, fn _, value ->
      value
      |> URI.parse()
      |> handle_parse()
      |> handle_error_parse()
    end)
  end

  defp handle_error_parse(error) when is_binary(error), do: [{field, Keyword.get(opts, :message, error)}]
  defp handle_error_parse(_), do: []

  defp handle_parse(%URI{scheme: nil}), do: "is missing a scheme (e.g. https)"
  defp handle_parse(%URI{host: nil}), do: "is missing a host"

  defp handle_parse(%URI{host: host}) do
    host
    |> Kernel.to_charlist()
    |> :inet.gethostbyname()
    |> handle_hostbyname()
  end

  defp handle_hostbyname({:ok, _}), do: nil
  defp handle_hostbyname({:error, _}), do: "invalid host"

I’m preferring to use Pattern Matching instead of case and other control flow structures.

what do you think?

1 Like
2

This is one of those cases of “Which do you prefer?” Your answer to that is the right one :smiley:

Personally I prefer multi-head functions whenever it’s possible, but that’s just me. I maybe wouldn’t start every function name with handle_ as it feels a bit noisy but, again, that’s just me.

2 Likes
3

I believe I heard once pattern matching is fast. So there’s that. but don’t listen to me, dummy here.

4

PS, pattern matching essentially is control flow (among other things). It’s right there on the Erlang homepage :slight_smile:

1 Like
5

A function with an anonymous function as an argument with a case inside that pipes into another case?

:melting_face:

to add something constructive: did you consider using with

6

IMO chained cases like this are better off as withs:

  defp validate_url(changeset, field, opts \\ []) do
    validate_change(changeset, field, fn _, value ->
      with {:ok, uri} <- parse_url(value),
           :ok <- check_host(url) do
        []
      else
        {:error, message} ->
          [{field, Keyword.get(opts, :message, error)}]
      end
    end
  end

  defp parse_url(value) do
    case URI.parse(value) do
      %URI{scheme: nil} ->
        {:error, "is missing a scheme (e.g. https)"}

      %URI{host: nil} ->
        {:error, "is missing a host"}

      uri ->
        {:ok, uri}
    end
  end

  defp check_host(%URI{host: host}) do
    case :inet.gethostbyname(to_charlist(host)) do
      {:ok, _hostent} -> :ok
      {:error, _posix} -> {:error, "invalid host"}
    end
  end

A particular thing I like about this approach is that changeset-related bindings stay in validate_url, versus having to be passed along to functions like handle_error_parse.

1 Like
7

both approaches are using pattern matching, my understanding is that a case statement and the equivalent pattern matching on function heads will compile to essentially the same byte code. i’ve done some very limited benchmarking and didn’t see any performance trade off for either approach

8

I personally prefer case over multiple function heads (sidenote: both use pattern matching!). But I’d do this:

defp validate_url_field(changeset, field, opts \\ []) do
  validate_change(changeset, field, fn _, value ->
    case validate_url(value) do
      :ok -> []
      {:error, message} -> [{field, Keyword.get(opts, :message, message)}]
    end
  end)
end

defp validate_url(string) do
  case URI.parse(string) do
    %URI{scheme: nil} ->
      {:error, "is missing a scheme (e.g. https)"}

    %URI{host: nil} ->
      {:error, "is missing a host"}

    %URI{host: host} ->
      case :inet.gethostbyname(Kernel.to_charlist(host)) do
        {:ok, _} -> :ok
        {:error, _} -> {:error, "invalid host"}
      end
  end
end
3 Likes
9

I prefer the multiple function heads. Because then i have scan lesser code to understand it.
The parent function looks pretty too. :stuck_out_tongue:

Only by reading the main function, i would know what is being done.
(assuming sensible naming, which is hard as they say)

I prefer understanding a program top to bottom. Especially , in a team. That is, don’t make me touch the details unless i have to.

1 Like
10

I am on the fence on this one. Sometimes I think the code is cleaner looking with a single function head that has a single case statement… :thinking:

Especially if the function has other heads that deal with other things. Sometimes the combinatory explosion flatting is just too much.

1 Like