Case with multiple values

I actually like the original version with private functions better.

Your context module will grow too big no matter how you organize its logic. I’d suggest to only have delegates and documentation in context module and delegate to multiple specific “private” modules that have actual logic, if you do that, then the size of each such module will not be a problem.

In my case, those private functions were only being used by the “do_something” function, and my module was growing quite big (it was a Phoenix context module)

But is it ideal to convert it from multi funs to case block / cond block? Or it’s just a matter of preference?

True, the names need correcting

There is no first, second, nor third in scope and it wasn’t bound so no, it will not work as expected. These were just another bindings for the attribute, bar, and baz respectively.

Of all the suggestions here, I find yours to be the clearest. At a glance you can understand what all the paths are; too many others leave something implicit to be reasoned out by a reader.

[EDIT] was replying to OP, in other words case {...} is clearest according to me.

I agree , my bad that I overlooked the return part as well so , but I guess there could be some problem with the default clause , as
in the case counterpart the external service will be called only when attribute matches with first , bar with second and baz with third so I believe to put an explicit check for them might be necessary , something like below

cond do
  baz == 12381 or length(bar) > 100 -> {:ok, :ignored}
  attribute == first and bar == second and baz == third -> ExternalService.call(attributes, bar, baz)
true -> <default behaviour>
end

This is one of the ways to do so, yes. However in case of case you do not need to pass first, so you could do:

case {bar, baz} do
  {_, 12381} -> {:ok, :ignored}
  {list, _} when length(list) -> {:ok, :ignored}
  _ -> ExternalService.call(attributes, bar, baz)
end

Or you could squash the two cases:

case {bar, baz} do
  {list, num} when num == 12381 or length(list) > 100 -> {:ok, :ignored}
  _ -> ExternalService.call(attributes, bar, baz)
end

However in that case whole pattern matching is needless, so we can do:

cond do
  baz == 12381 or length(bar) > 100 -> {:ok, :ignored}
  true -> ExternalService.call(attributes, bar, baz)
end

Or you could use if macro if it is clearer for you:

if baz == 12381 or length(bar) > 100,
  do: {:ok, :ignored},
  else: ExternalService.call(attributes, bar, baz)

EDIT:

@tushar as length(baz) is used only once then it is much better to not cache it, as if baz == 12381 the length(baz) will not be called at all which can save time, as counting length of the list can be needlessly expensive for long lists.

For your use-case , I would prefer cond

def do_something(attribute) do
  bar = get_bar(attribute)
  baz = get_baz(bar)


len = length(second)
  cond do
    baz==12381 -> {:ok, :ignored}
    len > 100 -> {:ok, :ignored}
    first==first and second==second and third==third -> ExternalService.call(first, second, third)
   true -> <your default behaviour if nothing matches>
  end
end