patternMinor
Generating all valid dates
Viewed 0 times
generatingvaliddatesall
Problem
I've come up with the following code to generate a list of all dates between two given dates:
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.
def generate_all_valid_dates_in_range(_start_date, _end_date) when _start_date Enum.to_list
|> Enum.map (&(:calendar.gregorian_days_to_date(&1)))
endI 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.
Solution
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:
And if you add
-
Your function would give a
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
-
You can neatify the last line to
All in all, I would transform it into something like this:
Note that the second guard (
Using the
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
@docattribute 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
_wordstyle is usually used for arguments that aren't used as part of the pattern matching. This is whyelixirdoesn't warn you if you haven't used an argument whose name starts with an underscore. I would rename tostart_dateandend_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 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"
endNote 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.Code Snippets
@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"
endContext
StackExchange Code Review Q#69120, answer score: 5
Revisions (0)
No revisions yet.