Hacker News Reader

Comment by midgetjones | Hacker News Reader

Great article, thanks!

I'm not an expert Alchemist by any means, but whenever I see a `cond`, I try to see if it can be refactored out using pattern matching (after I was picked up on the same thing during an interview!)

   defp formatted_hour("AM", "12"),  do: "00"
   defp formatted_hour("AM", hours), do: hours
   defp formatted_hour("PM", "12"),  do: "12"
   defp formatted_hour("PM", hours), do: 12 + String.to_integer(hours)

View on HN

I agree though one thing I would caution is it is good practice to always return the same type from a function even though it is an untyped language. The whole thing can be boiled down even more if pattern matching is used in parsing the initial string.

  def convert(<<h::bytes-size(2)>> <> ":" <>
              <<m::bytes-size(2)>> <> ":" <>
              <<s::bytes-size(2)>> <> <<period::bytes-size(2)>>) do
   "#{formatted_hour(h,period)}:#{m}:#{s}"
  end

  defp formatted_hour(hours,"PM"), do: "#{String.to_integer(hours) + 12}"
  defp formatted_hour("12","AM"), do: "00"
  defp formatted_hour(hours, "AM"), do: hours

Good point! If you're going to pattern match in the initial string then who even needs the second function?

    def convert("12:" <> <<m::bytes-size(2)>> <> ":" <> <<s::bytes-size(2)>> <> "AM") do
      "00:#{m}:#{s}"
    end

    def convert(<<h::bytes-size(2)>> <> ":" <> <<m::bytes-size(2)>> <> ":" <> <<s::bytes-size(2)>> <> "PM") do
      h = 12 + String.to_integer(h)
      "#{h}:#{m}:#{s}"
    end

    def convert(<<h::bytes-size(2)>> <> ":" <> <<m::bytes-size(2)>> <> ":" <> <<s::bytes-size(2)>> <> "AM") do
      "#{h}:#{m}:#{s}"
    end

Whether that's an improvement or not is another matter :)

Crazy stuff! :D

I'm wondering how our future-self would feel about such cleverness haha

I really liked this:

    "#{String.to_integer(hours) + 12}"

Used it now refactoring the code, many thanks for your suggestion!

I personally like recursive pattern matching

    defp formatted_hour("AM", hours), do: rem(hours, 12)
    defp formatted_hour("PM", hours), do: 12 + formatted_hour("AM", hours)

Nice!

Pattern matching is awesome and we should make use of it whenever appropriate, but not every situation is appropriate. For instance, if your formatted_hour functions were larger, or shared some amount of code between them, then cond might be a great option instead. Or case, or even the dreaded if statement. As a community, sometimes we overuse our cool special features just because we have them! Personally, I like the case statement as a middle ground for lots of scenarios. You still get pattern matching and it's pretty readable.

If the formatted_hour functions were larger or shared some amount of code, I wouldn't have suggested it ;)

Awesome! Many thanks for the tip. I'll refactor the code :)

Reply on news.ycombinator.com