Hey guys,
Excellent examples by Christopher and the fact they came up as a reply to a
misunderstanding on this thread shows exactly why I think they should be
added. I also agree that even those of us familiar with Elixir have those
moments where you totally forget and just fall back to using the comparison
operators alone, so it would be better to reinforce the concept with macros
in core. It's not necessary to have is_non_negative/1 because you can do
"when not is_negative(val)". If it absolutely comes down to choosing, I'd
say the Kernel ones would be the best due to 1) the import by default and
2) covering both numeric types (but as they're compile time, I really don't
see why not to include all).
Side note; I'm totally unfamiliar with the flow for suggesting features
here... what determines a "yes" vs a "no" (as this thread has both)? Does
it just get PR'd and rejected at that point, or?
Thanks everyone for your feedback!!
IW
On Tuesday, November 21, 2017 at 2:55:39 PM UTC-8, Christopher Keele wrote:
>
> 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/4e492803-0a11-4bcd-8634-85f99b47ee59%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
