>
> I would say no to this one. I think using < is ok. No change necessary.
>
> Elixir isnt statically typed, so at least for me, in general, i dont
> bother with is_number
>
As Issac points out, the comparison operators are not type-safe for this
use-case, though—just using < is *not* ok, as everything is comparable with
everything and "foo" < self() is perfectly valid. These guards aren't
syntactic sugar, they make the numeric is_xxx check precisely because it is
easy to forget that *v > 0 =/= is_positive(0)* and introduce subtle bugs
into your program:
def order_eggs(count) do
if count > 0 do
EggIO.order!(count)
else
raise ArgumentError, "you must order a positive number of eggs"
end
end
order_eggs(:asdf) #=>{:ok, %EggIO.Order{count: :asdf, status: :ordered}}
Or even:
def is_positive(value) when bar > 0, do: true
def is_positive(value) when bar <= 0, do: false
def is_positive(_), do: raise ArgumentError, "value is not a number"
foo(:asdf) #=> true
The weak typing of the comparison operators is useful when allowing any
term to be a key in an ordered data set. However in guard contexts it's
easy to forget, or assume that the comparison operators alone suffice for
positive number checks, especially as all the other Kernel operators are
strongly-typed:
"foo" ++ "bar" #=> ArgumentError
[] <> [] #=> ArgumentError
I think these guards would make a reasonable addition to core since they
could help prevent bugs and minimize a potential source of surprise to
newcomers. I know they would have spared me one confused afternoon of
debugging, since even though I know better I still sometimes I space out,
and even when I'm not thinking of type safety I like to use guards that
read nicely, so I would have unwittingly been accidentally protected from
this class of bug.
I lean on a soft no because we’d have to introduce six new functions as you
> said, plus is_non_negative likely as well
I think is_non_negative isn't necessary because it's common to match on the
zero case separately, and it's trivial to check it alongside the guard to
get the same effect: (eg is_positive(value) or value == 0)
Additionally we could choose between just the Kernel ones, or just the
namespaced ones. I feel as if the Kernel ones are more likely to be used
and therefore more likely to protect users from type-omissions bugs, so if
I had to only pick a subset I'd add them.
To be clear, those macros are used correctly within Timex, but that pattern
> is not safe in general, as the arguments are expanded multiple times.
That's a good point—any PR should use the recently-merged defguard which
was specifically implemented to handle this unquoting situation safely for
you.
--
You received this message because you are subscribed to the Google Groups
"elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/elixir-lang-core/d88a852f-eb19-47f7-b646-899b388ff939%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
