Generating all valid dates

Question

I've come up with the following code to generate a list of all dates between two given dates:

   def generate_all_valid_dates_in_range(_start_date, _end_date) when _start_date <= _end_date do
    (:calendar.date_to_gregorian_days(_start_date) .. :calendar.date_to_gregorian_days(_end_date))
    |> Enum.to_list
    |> Enum.map (&(:calendar.gregorian_days_to_date(&1)))
  end

I would appreciate any comments on style and if there's some built in function (in either Elixir or Erlang) to achieve this sort of functionality.

Answer 1

I'm probably no better in Elixir than you are, but I will give it a go.

First of all the good parts: I think the function does what it should, and it's fairly neat. Not bad for a language that doesn't even have a concept of date, huh? :-)

Suggestions for improvement:

• You should have a @doc attribute and unit tests :-) Remember to document that the range is inclusive (which is not what I would expect). And if you add @doc, consider adding an example which can be run via a doctest in ExUnit. More info: elixir-lang.org/docs/stable/ex_unit/ExUnit.DocTest.html (thanks to @alxndr).
• Consider making the range not inclusive, but rather [from, to) or (from, to]. The former is common in computer science, the latter is common in daily speech.
• The _word style is usually used for arguments that aren't used as part of the pattern matching. This is why elixir doesn't warn you if you haven't used an argument whose name starts with an underscore. I would rename to start_date and end_date.
• I would introduce an anonymous function working as an alias, to make the first line a bit easier on the eye. See example below.

• Your function would give a MatchError on invalid output. I would probably deliberately check it and raise an ArgumentError instead.

  Update: The Erlang/Elixir philosophy is let it fail, and therefore this suggestion might be contrary to common E/E style. From the creator himself:

  This is typically how Elixir/Erlang code is written indeed. If a condition is not met, it fails as MatchError or FunctionClauseError. We typically don't add an extra clause saying what went wrong. The upside is that we simply worry about the happy path (forcing us to write more assertive code). The downside is that it may be cryptic sometimes to find exactly what went wrong. We do include the arguments in the stacktrace though. — José Valim

• You can neatify the last line to |> Enum.map &:calendar.gregorian_days_to_date/1.

All in all, I would transform it into something like this:

@doc """
Insert insightful documentation here.
"""
def generate_all_valid_dates_in_range(start_date, end_date)
when start_date <= end_date do
  to_days = &:calendar.date_to_gregorian_days/1
  to_days.(start_date) .. to_days.(end_date)
  |> Enum.to_list
  |> Enum.map &:calendar.gregorian_days_to_date/1
end

def generate_all_valid_dates_in_range(start_date, end_date)
when start_date > end_date do
  raise ArgumentError, "start_date must be before end_date"
end

Note that the second guard (when) is there only to highlight what the invariant is. It should catch all calls that the first definition doesn't. If any calls "fall through", we will get a MatchError and know there's something wrong in our logic.

Using the to_days alias, including the guard in the latter function definition and wrapping the guards to make the lines shorter are a matter of taste, and therefore optional.