You could be a little more terse with:

defp parse_attr(:body, value),
  do: value

defp parse_attr(:tags, value),
  do: value |> String.split(",") |> Enum.map(&String.trim/1) |> Enum.sort()

defp parse_attr(_, value),
  do: String.trim(value)

That does the same thing as your first example except that it will not throw an error if something other than :title, :author, :description, :body or :tags is passed as the first argument to that function. In that case it just returns String.trim(value). You could make it throw an error by using a guard (see the example below), and not defining a function clause that matches the first argument on anything.

The code below is what I think you intended to do with your second example, but there are two differences between this and that example:

1. It returns value if the first argument is :body (the second example would return String.trim(value))
2. It returns “Oops!” if the first argument wasn’t one of the expected ones (the second example returns String.trim(value) if the first argument is anything other than :tags).

The third clause in that case block will never match because :error would match on the second clause.

defp parse_attr(:body, value),
  do: value

defp parse_attr(:tags, value),
  do: value |> String.split(",") |> Enum.map(&String.trim/1) |> Enum.sort()

defp parse_attr(trimmed, value) when trimmed in [:title, :author, :description] do
  String.trim(value)
end

# If you leave this out, calls to this function will raise an error if the first 
# value is not one of the expected ones above.
defp parse_attr(_, value),
 do: "Oops!"

As the other poster said, this is pretty idiomatic Elixir code. I felt like it would be confusing when I started learning Elixir, but now I find it much easier to read and reason about than one or more control flow blocks.

Another way to clean up this code without changing any semantics of how it works would be to use guard clauses:

defp parse_attr(attr, value) when attr in [:title, :author, :description], 
  do: String.trim(value)

defp parse_attr(:body, value),
  do: value

defp parse_attr(:tags, value),
  do: value |> String.split(",") |> Enum.map(&String.trim/1) |> Enum.sort()

This keeps the functionality intact where this function will reject if not called with one of the specific atoms defined in the function head, but it’s slightly less code to write for each case.

Edit: did not originally see the second part of @srowley’s example, they suggested this approach as well. Anyway, this is how I’d do it.

Hello and welcome,

This post might give You some idea…