Improve the error message when the wrong number of arguments are provided #212
Conversation
There was a problem hiding this comment.
Sorry, I'm late to this PR as I was on leave.
First of all, thanks for your contribution! We'd love to know more about how you're using this library and if you're finding it useful.
To answer your questions about v2: it is on hold and potentially to be removed entirely. It was an attempt at a re-implementation to address some issues but we're not pursuing those at the moment. It's not receiving any active development, feel free to remove any of the changes you made to v2 (and reminding me that we need to mothball it).
On the function_exported?, let's try the Module.__info__() approach and see if that's a little less brittle?
lib/expression/callbacks.ex
Outdated
| # Otherwise fail | ||
| true -> | ||
| {:error, "#{function_name} is not implemented."} | ||
| end | ||
| end | ||
|
|
||
| defp wrong_arity_but_function_exists?(module, function_name) | ||
| when is_atom(module) and is_atom(function_name) do | ||
| Enum.any?(0..20, fn arity -> Kernel.function_exported?(module, function_name, arity) end) |
There was a problem hiding this comment.
I like the pragmatism here and I've not actually done this myself before but perhaps Module.__info__()[:functions] is a way to do this? The docs suggest that returns a list of functions and their arities so maybe that's worth cross referencing?
There was a problem hiding this comment.
That's much better, thank you!
TBH the Enum.any?(0..20, ... was a ChatGPT suggestion - I'm still an Elixir newbie so great to learn this!
…ided Currently when a user provides the wrong number of arguments, they receive a message saying `echo is not implemented.`. This message is confusing because it makes the user feel like they typed in the wrong function name, when in reality they just provided the wrong number of arguments. This is a first draft at a fix - feedback welcome!
metade
force-pushed
the
better-error-for-wrong-number-of-arguments
branch
from
ec616d8
to
74ae0ab
Compare
|
Thank you for the feedback! I hope you had some lovely time off on your leave!
I'm actually @lorenzosinisi's colleague, we're building a back office tool where
Great, that makes sense - I removed those changes.
I have done this - looking forwards to the next review. |
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA 1 out of 2 committers have signed the CLA. |
Currently when a user provides the wrong number of arguments, they receive a message saying
#{function_name} is not implemented..This message is confusing because it makes the user feel like they typed in the wrong function name, when in reality they just provided the wrong number of arguments.
Instead, a message such as
wrong number of arguments to #{function_name}is more helpful to the user.This is a first draft at a fix - feedback welcome!
Thins I'm not sure about:
Enum.any?(0..20, fn arity -> Kernel.function_exported?(module, function_name, arity) end)wrong_arity_but_function_exists?isn't duplicated