Proper way of handling simple booleans in `with` statement

Just to throw some grenades, Jose on tagged with on the ThinkingElixir Podcast and relatedly keathleys Good Bad Elixir.

See also the beware section in the with docs, and a similar question I directed at Saša Jurić.

Jose’s argument (and basically every other “anti-tagger” I have read/heard) is that with is explicitly made to handle the case [sic] where your failure states are pretty unified. “If the error matters, use case” sticks in my head.

My take away is that the else is actually almost an after thought for some edge cases, and that most uses of with probably shouldn’t need/have one.

Do I agree with this? Not really sure. I dont really like nested case statements:

case is_integer(foo) do
  true ->
    case rem(foo,2) == 0 do
      true ->
          case 
          ...  # blergh :vomit:

You could wrap each step in its own ensure_integer, ensure_rem2 function, etc that return :ok | {:error, :x} but that feels like a lot of work for some simple checks.

You could write

def validate_foo(foo) when is_integer(foo) and rem(foo,2) == 0 and foo > 50 do
:ok # or act_foo(foo), whatever
end

def valiate_foo(foo) when is_integer(foo) and rem(foo, 2) == 0 do
  {:error, :under_50}
end


def valiate_foo(foo) when is_integer(foo) do
  {:error, :not_rem_2}
end

but that’s not great either IMO because the logic becomes pretty complicated to keep track of.

I think some of the problem is we reduce these problems down to talk about them when really that abstracts any ability to reason on why we should or shouldn’t use a form. In this case does it matter if your foo is bad? can you recover? is passing {:error, reason} useful?

It’s trite, but “know the rules to break the rules” is a valid idiom.

In some cases, tagging the tuple is just the most ergonomic way to do it. In other cases you may want to wrap it in a data structure (such as using Ecto changesets, which are great outside of Ecto) or use a more structured set of functions or case statements.

I would posit that if you want to return a reasonable error back to the user (i.e. they provided a foo of bad quality), then building a changeset or similarly robust structure around it isn’t a bad idea, because you’re now talking about a business rule which can probably stand to be codified more concretely.

Otherwise perhaps you really only need to return {:error, :invalid_foo} and the UI should just be explicitly stating the qualifiers around foo (must be even, must be over 50, etc).

I do like both eksperimentals x || :error and kartheeks error_* styles.

14 Likes

My solution kinda feels like enterprise fizzbuzz when compared to the other
ones here, but I’ve seen this problem be solved with something along the lines
below. In my case, we log a lot and use metrics a ton, so this approach is biased
towards that.

This approach helps with guiding where the checks go (private function that
respects an ok/error tuple response), which type of message should be logged
and kind of metric you should push up.

The embedded_schema approach @kip mentioned above is also a good one.

def process(foo) do
  with {:ok, _} <- check_integer(foo),
       {:ok, _} <- check_divisible(foo, 2),
       {:ok, _} <- check_greater_than(foo, 50),
       # I'd add your "Do something" call here as well
       {:ok, _} = success <- Something.call(foo) do
    Loger.info("Success!")
    Metrics.increment("something.process.successes")
    success
  else
    {:error, %{message: msg, error_kind: error_kind}} = error ->
      Logger.error("Error processing #{foo}, error message: #{msg}")
      Metrics.increment("something.process.errors", %{tags: [error_kind]})
      error
    error ->
      Logger.error("Error processing #{foo}, unexpected error: #{inspect(error)}")
      Metrics.increment("something.process.errors", %{tags: ["kind:unexpected"]})
      error
  end
end
  
defp check_integer(foo) do
  case is_integer(foo) do
    true -> {:ok, foo}
    _ -> {:error, %{message: "Integer error: #{foo} is not an integer", error_kind: "kind:check_integer"}}
  end
end

defp check_divisible(foo, divisor) do
  case rem(foo, divisor) == 0 do
    true -> {:ok, foo}
    _ -> {:error, %{message: "Divisibility error: #{foo} is not divisible by #{divisor}", error_kind: "kind:check_divisible"}}
  end
end

defp check_greater_than(foo, target) do
  case foo > target do
    true -> {:ok, foo}
    _ -> {:error, %{message: "Comparison error: #{foo} is not greater than #{target}", error_kind: "kind:check_greater_than"}}
  end
end

I thought I should share this even though it is way more verbose than the other solutions here.

6 Likes

In fairness, your example does look very much like validation. What is it you want to do when a branch fails?

1 Like

Great example, especially with the included real world aspects of logging/metrics. Also shows a great use of else where it’s not trying to catch every possible case, just some particular, well defined wrappers. Only looks like enterprise fizzbuzz because we’re talking in such abstracted terms, in real use this would look very appropriate.

3 Likes

I just realised that else is not needed in my previous post.

def do_something(foo) do
     with :ok <- error_unless(is_integer(foo), :non_integer),
          :ok <- error_unless(rem(foo,2) == 0, :not_even),
          :ok <- error_unless(foo > 50, :foo_gt_50) do
     # do something
     end
end

I find error_* functions very handy in ErrorUtils to wrap bool with :ok, {:error, msg }

defmodule ErrorUtils do
  def error_if(true, msg), do: {:error, msg}
  def error_if(_, _), do: :ok

  def error_unless(false, msg), do: {:error, msg}
  def error_unless(_, _), do: :ok

  def error_if_nil(nil, msg), do: {:error, msg}
  def error_if_nil(_, _), do: :ok

  def error_if_empty(enumerable, msg) do
    case Enum.empty?(enumerable) do
      true ->
        {:error, msg}

      false ->
        :ok
    end
  end

end
2 Likes

My preferred approach for this is to have a generic validate helper which converts boolean into :ok | {:error, reason}:

def validate(true, _reason), do: :ok
def validate(false, reason), do: {:error, reason}

And now with can be expressed as:

with :ok <- validate(is_integer(foo), :not_an_integer),
     :ok <- validate(...),
     ...,
     do: ...

I briefly discussed this in this blog post.

24 Likes

Cursed answer:

def validate(foo) do
  is_integer(foo) or throw :not_integer
  rem(foo,2) == 0 or throw :not_even
  foo > 50 or throw :too_big
catch
  :not_integer -> ...
  :not_even -> ...
  :too_big -> ...
end
10 Likes

Yep, composability. I like this one the most.

4 Likes

Get this Java sorcery away from my sight!

1 Like

Oh I like this pattern for simple booleans, it’s so simple, even more so than what I generically do! I love it!

This is what I usually do yeah. It’s nice because it works on any return type, not just booleans.

Yeah I have a few of these as well for more purpose specific cases.

Lol, wonder how much overhead that gives, hmm…

2 Likes

Probably not much. If I’m not mistaken, throw is not as bad as raise

Raise allocates a map so yeah, but I’m more comparing to how it would compare to the with, hmm, let’s do a super quick but probably incorrect benchmark (specifically built the raise version to use hardcodeable exception values so it shouldn’t cost anything extra for the allocation as it will reference the constant pool):

defmodule WithThrowBench do
  def classifiers(), do: nil

  def time(_), do: 2

  def inputs(_cla),
    do: %{
      "" => 1..100,
    }

  def setup(_cla), do: nil

  def teardown(_cla, _setup), do: nil

  def validate_throw(foo) do
    is_integer(foo) || throw :not_integer
    rem(foo, 2) == 0 || throw :not_even
    foo > 50 || throw :too_big
    true
  catch
    :not_integer -> 1
    :not_even -> 2
    :too_big -> 3
  end

  defmodule NotIntegerError do
    defexception message: "not an integer"
  end
  defmodule NotEvenError do
    defexception message: "not an even number"
  end
  defmodule TooBigError do
    defexception message: "number is too big"
  end

  def validate_raise(foo) do
    is_integer(foo) || raise %NotIntegerError{}
    rem(foo, 2) == 0 || raise %NotEvenError{}
    foo > 50 || raise %TooBigError{}
    true
  rescue
    NotIntegerError -> 1
    NotEvenError -> 2
    TooBigError -> 3
  end

  def validate_with(foo) do
    with true <- is_integer(foo) || :not_integer,
         true <- rem(foo, 2) == 0 || :not_even,
         true <- foo > 50 || :too_big do
      true
    else
      :not_integer -> 1
      :not_even -> 2
      :too_big -> 3
    end
  end

  def actions(_cla, _setup), do: %{
    "with" => fn vs -> Enum.map(vs, &validate_with/1) end,
    "throw" => fn vs -> Enum.map(vs, &validate_throw/1) end,
    "raise" => fn vs -> Enum.map(vs, &validate_raise/1) end,
  }
end

And results:

❯ mix bench with_throw
Operating System: Linux
CPU Information: AMD Phenom(tm) II X6 1090T Processor
Number of Available Cores: 6
Available memory: 15.63 GB
Elixir 1.10.0
Erlang 22.2.5

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 2 s
memory time: 2 s
parallel: 1
inputs: 
Estimated total run time: 18 s


Benchmarking raise with input ...
Benchmarking throw with input ...
Benchmarking with with input ...
##### With input  #####
Name            ips        average  deviation         median         99th %
with        92.77 K       10.78 μs    ±21.07%          10 μs          16 μs
throw       53.77 K       18.60 μs    ±80.95%          18 μs          25 μs
raise       41.96 K       23.83 μs    ±70.59%          23 μs          34 μs

Comparison: 
with        92.77 K
throw       53.77 K - 1.73x slower
raise       41.96 K - 2.21x slower

Memory usage statistics:

Name     Memory usage
with          1.81 KB
throw        11.63 KB - 6.42x memory usage
raise        11.63 KB - 6.42x memory usage

**All measurements for memory usage were the same**
3 Likes

Let’s see how much each type actually costs, so changed the inputs to this:

  def inputs(_cla),
    do: %{
      "valid" => Enum.filter(0..50, &(rem(&1, 2) == 0)),
      "invalid-0" => Enum.map(0..25, fn _ -> "not integer" end),
      "invalid-1" => Enum.filter(0..50, &(rem(&1, 2) == 1)),
      "invalid-2" => 51..76,
    }

And results:

❯ mix bench with_throw          
Operating System: Linux
CPU Information: AMD Phenom(tm) II X6 1090T Processor
Number of Available Cores: 6
Available memory: 15.63 GB
Elixir 1.10.0
Erlang 22.2.5

Benchmark suite executing with the following configuration:
warmup: 2 s
time: 2 s
memory time: 2 s
parallel: 1
inputs: invalid-0, invalid-1, invalid-2, valid
Estimated total run time: 1.20 min


Benchmarking raise with input invalid-0...
Benchmarking raise with input invalid-1...
Benchmarking raise with input invalid-2...
Benchmarking raise with input valid...
Benchmarking throw with input invalid-0...
Benchmarking throw with input invalid-1...
Benchmarking throw with input invalid-2...
Benchmarking throw with input valid...
Benchmarking with with input invalid-0...
Benchmarking with with input invalid-1...
Benchmarking with with input invalid-2...
Benchmarking with with input valid...
##### With input invalid-0 #####
Name            ips        average  deviation         median         99th %
with       857.40 K        1.17 μs   ±485.73%        1.10 μs        2.10 μs
throw      159.86 K        6.26 μs   ±191.15%           6 μs          11 μs
raise      123.96 K        8.07 μs   ±299.72%           8 μs          14 μs

Comparison: 
with       857.40 K
throw      159.86 K - 5.36x slower
raise      123.96 K - 6.92x slower

Memory usage statistics:

Name     Memory usage
with          0.90 KB
throw         3.71 KB - 4.13x memory usage
raise         3.71 KB - 4.13x memory usage

**All measurements for memory usage were the same**
##### With input invalid-1 #####
Name            ips        average  deviation         median         99th %
with       536.45 K        1.86 μs    ±95.17%        1.80 μs        2.70 μs
throw      143.36 K        6.98 μs   ±145.59%           7 μs          10 μs
raise      117.37 K        8.52 μs   ±210.54%           8 μs          14 μs

Comparison: 
with       536.45 K
throw      143.36 K - 3.74x slower
raise      117.37 K - 4.57x slower

Memory usage statistics:

Name     Memory usage
with          0.87 KB
throw         3.58 KB - 4.13x memory usage
raise         3.58 KB - 4.13x memory usage

**All measurements for memory usage were the same**
##### With input invalid-2 #####
Name            ips        average  deviation         median         99th %
with       307.61 K        3.25 μs   ±800.67%           3 μs           5 μs
throw      219.04 K        4.57 μs   ±363.08%           4 μs           8 μs
raise      183.71 K        5.44 μs   ±289.52%           5 μs          12 μs

Comparison: 
with       307.61 K
throw      219.04 K - 1.40x slower
raise      183.71 K - 1.67x slower

Memory usage statistics:

Name     Memory usage
with          0.95 KB
throw         2.26 KB - 2.37x memory usage
raise         2.26 KB - 2.37x memory usage

**All measurements for memory usage were the same**
##### With input valid #####
Name            ips        average  deviation         median         99th %
with       424.42 K        2.36 μs    ±56.56%        2.30 μs        3.10 μs
throw      136.27 K        7.34 μs   ±147.99%           7 μs          11 μs
raise      100.66 K        9.93 μs    ±75.73%          10 μs          15 μs

Comparison: 
with       424.42 K
throw      136.27 K - 3.11x slower
raise      100.66 K - 4.22x slower

Memory usage statistics:

Name     Memory usage
with          0.90 KB
throw         3.71 KB - 4.13x memory usage
raise         3.71 KB - 4.13x memory usage

**All measurements for memory usage were the same**
5 Likes

I’m surprised throw is so bad in the happy path! Guess I learned something today!! Thanks

5 Likes

I’m guessing because it has to encode the stack frame or so. I’d have figured the valid/happy path to be about the same speed on them all, guess that shows how expensive stack frames are.

5 Likes

catching and rescueing is really expensive. In Elixir core It is absolutely discouraged unless really necessary

3 Likes

In our projects we’ve basically replaced all usages of with with happy_with, wich IMO is a strictly superior form of with (in fact, if you want, you can just use it as a standard with with less anacronystic syntax)

happy_with do
  @integer true <- is_integer(foo)
  @even true <- rem(foo,2) == 0
  @large_enough true <- foo > 50
  # do something
else
  {:integer, _} -> {:error, "foo must be an integer"}
  {:even, _} -> {:error, "foo must be even"}
  {:large_enough, _} -> {:error, "foo must be larger than 50"}
end
4 Likes

We too :wink:

withl label1: result1 <- expr1(),
      label2: result2 <- expr2() do
  do_something(result1, result2)
else
  label1: error -> handle_error1(error)
  label2: error -> handle_error2(error) # note that result1 is accessible here in case you need it
end
3 Likes

I’m not trying to be pedantic, but your example does seem like ‘validation’ – a lot of the standard Ecto.Changeset validations are “simple boolean check[s]”. Your checks may or may not fit ‘changeset validations’ even better depending on how or whether your simple checks ‘accumulate’ (or not), i.e. would it be better for callers to know all of the checks that have failed or do you ‘really want’ to ‘fail’ at a specific check (performed in a specific order) or would it be better to perform all of the checks and ‘report’ all of those that have failed?

In your original example, I’d guess that it’d be better to report all of the failed checks, e.g. so a user (or caller) wouldn’t have to repeatedly call whatever code contains your with until all checks pass.

1 Like

you can ignore the example in its wholeness. it’s not real code, it’s something I wrote down solely when writing the initial post.

the sole purpose was to illustrate some case where among other various statements (fetch data, transform it, count it,…), I need to make a ultra simple boolean check like true <- foo.count < 30 || :capacity_full and deal with that roadblock in my happy path. so my real code is not just a series of 3 boolean checks that should be returned together.

sorry if the example was misleading in this respect. but it seems the discussion covers whole range of approaches, from simple ones like the one I choose for now, up to more generic approaches for more complex use cases.

everyone: one thing that surprised me was the recommendation that else block should be ideally empty or have one “capture all” error. on the other hand, most of the examples here do use the else block to hadle various types of errors separately. what gives? :slight_smile:

3 Likes