Am I doing multiple clauses right?

Questions / Help
,
1

This is very un-DRY, and I haven’t even put in a @spec yet! What would be an idiomatic refactoring?

defmodule PublicLawWeb.LayoutView do
  use PublicLawWeb, :view
  alias Backpack.Inflex

  def html_title(%Plug.Conn{assigns: %{subdomain: "www"}}), do: "Public Law"

  def html_title(%Plug.Conn{assigns: %{subdomain: s}}) do
    "#{s} public law"
    |> String.trim()
    |> Inflex.titleize()
  end

  def html_title(%Plug.Conn{}), do: "Public Law"
end

I figured I should ask while my code is very simple, and I’m still learning good style.

2

I tend to use many small help functions to make things clearer. I would do something like this:

def html_title(%Plug.Conn{assigns: %{subdomain s}}), do: domain_to_title(s)

defp domain_to_title(s) when not s, do: "Public Law"
defp domain_to_title("www"), do: "Public Law"
defp domain_to_title(s), do: "#{s} public law" |> String.trim() |> Inflex.titleize()

or if you rather like the case way:

defp domain_to_title(s) do
    case s do
       s when not s -> "Public Law"
       "www" -> "Public Law"
       s ->  "#{s} public law" |> String.trim() |> Inflex.titleize()
    end
end

You can combine the first and second thingy and use an or in the guard but I think this reads better.

1 Like
3

I like the lack of redundancy in the case, but I if it’d be possible for the “happy path” to come first; that the site title is the subdomain + " Public Law"…

I’m kind of weighing the value of declarative code for readability. Maybe if I stop and think about what my algorithm is:

The HTML title is [Subdomain] Public Law when a subdomain is present and it’s not www.
Otherwise, the title is simply Public Law.

With that in hand, I can try the approach, coding-to-make-the-algorithm-visible. Something like this pseudo-code:

def html_title(conn)
    "#{conn.subdomain} Public Law".titleize
    when
        conn.subdomain.present? and conn.subdomain != "www"
    else
        "Public Law"
end

And what would be idiomatic Elixir for that?

4

My next iteration:

  def html_title(%Plug.Conn{} = conn) do
    case conn do
      %{assigns: %{subdomain: s}} when s != "www" ->
        "#{s} public law" |> Inflex.titleize()

      _ ->
        "Public Law"
    end
  end

I really like that a guard can be used with a match clause!

5

And a final iteration, by refactoring the case to multiple clauses:

  def html_title(%Plug.Conn{assigns: %{subdomain: s}}) when s != "www" do
    "#{s} public law" |> Inflex.titleize()
  end

  def html_title(%Plug.Conn{}), do: "Public Law"

I like this version the best - less nesting, seems to require less mental overhead to understand.

2 Likes
6

I think this is exactly where you needed to end up. It’s split up to neatly have you edit the cases so that when you add one it’s just a new function clause and you know you won’t be making incisions into the other cases, it explains the exact logic of what you’re trying to do clearly and best of all, no case on raw function parameters, my biggest pet peeve… :smiley:

1 Like