Using a literal parameter to choose between multiple function heads

roman

Hi,
Imagine we have a code like this in a language that doesn’t support functions with multiple heads:

def parse_date(format, value) do
  case format do
  :iso8601 -> Date.from_iso8601(value)
  :american -> value |> String.split("/") |> …
  end
end

If we always pass the format as an atom literal, i.e. parse_date(:iso8601, value), this is considered an anti-pattern (Flag Argument).

In Elixir, however, this is not so clear, because the implementations are more separated and there’s no explicit branching:

def parse_date(:iso8601, value) do
  Date.from_iso8601(value)
end

def parse_date(:american, value) do
  value |> String.split("/") |> …
end

Do you consider this an anti-pattern in Elixir as well, and we should have differently named functions? Or does this approach fit the general pattern-matching style of Elixir?

7 comments

#code-style #refactoring

0 1585 7

2020-11-19 14:39:06 UTC

Showing Posts 1 to 7

Show Best Posts
Show All (oldest first)
Show All (newest first)

rcavanaugh

I view that as kinda similar to the way Objective-C makes it’s named arguments part of the method name.

NSString initWithData:encoding:

Post #1

al2o3cr

An initial style note: the first argument position is usually reserved for the “subject” - value here - so that pipes work nicely:

some
|> chain()
|> of_expressions(that: return_a_string_shaped_like_a_date)
|> parse_date(:american)

Whether this is better as |> parse_date(:american) or parse_date_american() depends on usage; is the format always hard-coded or is it driven by data (for instance, from a user preference)?

Another deciding factor might be the implementation: are the two function heads independent, or do they rely on common private functions?

Post #2

zkessin

Normally you would put the value first, but I don’t think that is in any way an anti pattern. It also makes testing easier.

Post #3

roman

the first argument position is usually reserved for the “subject” - value here - so that pipes work nicely

That’s a very good point. I think it alone is enough to avoid this style.

Whether this is better as |> parse_date(:american) or parse_date_american() depends on usage; is the format always hard-coded or is it driven by data (for instance, from a user preference)?

The format is always hard-coded, that’s a key part of the question.

are the two function heads independent, or do they rely on common private functions?

Could you expand on that? I didn’t see it as a relevant factor.

Post #4

roman

Do you mean that we can put the names in a list [:iso8601, :american] and generate tests iteratively?

Post #5

LostKobrakai

I’ve looked into the arguments of martin fowler:

Tangled Implementation

This can happen, but given in elixir we usually write different function bodies like he does for two different functions I feel this is is not more of an issue then for his proposed solution.

Deriving the flag

Given we’re not in oop land, somewhere some functions needs to decide for which “flag” to use – only then parse_date is called. So most of the discussion is not applicable to elixir. But I’m with martin that boolean flags are bad. I don’t want to see calls like parse_date(true, date) vs. parse_date(false, date) to differenciate :america | :iso8601. Either use atoms describing the flag or if it’s truely boolean use a keyword list: parse_date(value, time_only: true)

Post #6

al2o3cr

Here’s an example of “two heads” with overlapping implementation:

https://github.com/elixir-lang/elixir/blob/7533e56c1baec77c2aa1a4c0ad02730fae464201/lib/elixir/lib/calendar/date.ex#L348-L359

The heads here are matching on an atom (Calendar.ISO) inside the input struct, and the second one depends directly on the first. Making them separate functions would obscure that coupling.

The function called to do the work, Calendar.ISO.date_to_string is another example of this style:

https://github.com/elixir-lang/elixir/blob/7533e56c1baec77c2aa1a4c0ad02730fae464201/lib/elixir/lib/calendar/iso.ex#L899-L911

date_to_string is only responsible for type guards and delegates its work to a helper function. Passing the format as an argument instead of having date_to_string_basic and date_to_string_extended helps here, because the decision about which format to use (in the code calling to_iso8601) is several layers of function calls away from where the code branches (date_to_string_guarded’s two heads).

Post #7

— All posts loaded —