Exploit "Errors in guards" in defguard

I have this helper function where I exploit the Errors in Guards feature.

def non_empty(val, _msg) when length(val) > 0, do: {:ok, val}
def non_empty(val, _msg) when map_size(val) > 0, do: {:ok, val}
def non_empty(val, _msg) when tuple_size(val) > 0, do: {:ok, val}
def non_empty(_val, msg), do: {:error, msg}

(I can call non_empty({:foo}, "") though length({:foo}) raises)

Is there a way to put this into a defguard - without using is_list/1 etc?

defguard enum_size... ???
1 Like

size is constant time O(1), where as length is linear time -O(N) - Naming Conventions — Elixir v1.13.2

Isn’t non_empty for lists a bit more expensive operation - depending on size of list?

Enum.empty?/1 just compares to [] when it is list.

def empty?(enumerable) when is_list(enumerable) do
    enumerable == []
end

Maybe your non_empty can be changed to this ?

def non_empty(val, _msg) when map_size(val) > 0, do: {:ok, val}
def non_empty(val, _msg) when tuple_size(val) > 0, do: {:ok, val}
def non_empty([], msg), do: {:error, msg}
def non_empty(val, _msg) when is_list(val), do: {:ok, val}
def non_empty(_val, msg), do: {:error, msg}

or something like this only if you pass enumerable or tuple, else this will return {:ok, val} for strings, numbers, etc

  def non_empty([], msg), do: {:error, msg}
  def non_empty({}, msg), do: {:error, msg}
  def non_empty(%{} = val, msg) when map_size(val) == 0, do: {:error, msg}
  def non_empty(val, _msg), do: {:ok, val}
2 Likes

This is how I would rewrite your clauses into just two.

  def non_empty(val, _msg)
      when is_list(val) and val != []
      when is_map(val) and map_size(val) > 0
      when is_tuple(val) and tuple_size(val) > 0 do
    {:ok, val}
  end

  def non_empty(_val, msg), do: {:error, msg}

by checking for the type, it does not raise (anyway that is not a problem if you only have one when clause), but in my example it is needed, othewise later when clauses will never be evaluated if any prior clause raises.
Personally I consider good practice to check for types.

3 Likes

please don’t! It totally changes the logic of the function, unless you are fine with it.
As mentioned by @karthheek, it will return {:ok, } in case of an uncovered type.

non_empty(0, "it is empty") will return {:ok, 0}

2 Likes

Why don’t you want to use is_list/1 and what is the issue with raising within the guard?

2 Likes

or you can make it a guard, it will not work for structs though.

  defguard is_non_empty(term)
           when (is_list(term) and term != []) or
                  (is_map(term) and map_size(term) > 0) or
                  (is_tuple(term) and tuple_size(term) > 0)

2 Likes

non_empty function using defguards:

defguard is_non_empty(enum) when ((enum == []) or (enum == {}) or (is_map(enum) and map_size(enum) == 0)) == false

def non_empty(val, _msg) when is_non_empty(val), do: {:ok, val}
def non_empty(_, msg), do: {:error, msg}

@eksperimental :+1: .

2 Likes

@kartheek, that is definitely the solution to the question by the OP

2 Likes

@eksperimental I feel both our solutions are very similar. Fun collaborating with you.

@Sebb I was running some benchmarking (just for fun) for below data. Your existing version was taking hit when data is non empty list. You may have to change it.
Which version to use ? you have to figure out by running benchmark or based on other requirements of in the project where it is used.

empty_list = []
list1 = Enum.to_list(1..3)
list2 = Enum.to_list(1..10)
list3 = Enum.to_list(1..100)
empty_map = %{}
map = %{1 => 2}
empty_tuple = {}
tuple = {1}

Benchee.run(%{
  "non_empty_empty_list"    => fn -> TestPattern.non_empty(empty_list, "") end,
  "non_empty_list1"    => fn -> TestPattern.non_empty(list1, "") end,
  "non_empty_list2"    => fn -> TestPattern.non_empty(list2, "") end,
  "non_empty_list3"    => fn -> TestPattern.non_empty(list3, "") end,
  "non_empty_empty_map"    => fn -> TestPattern.non_empty(empty_map, "") end,
  "non_empty_map"    => fn -> TestPattern.non_empty(map, "") end,
  "non_empty_empty_tuple"    => fn -> TestPattern.non_empty(empty_tuple, "") end,
  "non_empty_tuple"    => fn -> TestPattern.non_empty(tuple, "") end,
})
2 Likes

@kartheek definitely yours is more performant.

Here’s another version I think even simpler, that builds on your idea.
defguard is_non_empty(term) when term not in [{}, [], %{}]

3 Likes

This is simpler :+1: This will be slower than your original defguard.

As long as time complexity of expression is constant time O(1) - more or less same performance. Your versions are constant time too, so they will have similar performance.

In benchmark - your original defguard is performing the best :+1: .


Is better performance due to integer comparison vs equality operators?

1 Like

Wow, I am surprised by the result,
it could be that is_list(term) is faster than term == [], and so on with the other is_* functions. We could benchmark these.

The other thing is that it is so fast than the results are imprecise.
If the results are correct, then it pays off to always check for the type as I was doing.

2 Likes

Way too clever for my taste. I would not approve that in a PR and would prefer @eksperimental’s solution.

2 Likes

@eksperimental :+1:

@dimitarvp I understand your point.

I am not criticising OP. I don’t know specifics of how non_empty function is being used.

I feel original non_empty function is little over engineered - covers list, map, tuple.

Most of the time, result tuple is pattern matched. Tuples are rarely iterated like list.

I use Enum.empty? Or [] for empty checks. I have some error_* which return error messages for a Boolean expression.

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

ErrorUtils module has error_unless, error_if_nil, error_if_not_nil, etc.

Using these error funcs, with and case - I never felt need for anything else.

I wrote defguard for the first time today. This non_empty function does not need defguard as such.

1 Like

Sorry guys, I had a rough time and completely forgot about this thread.

Thanks for the insights.

As it seems we can’t exploit “errors in guards” inside a defguard, so I’ll go with your proposal.

2 Likes