-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Identify optional arguments in callback functions #981
Comments
I propose as follows:
|
Unfortunately the syntax |
Point 4, silently dropping arguments in Did you intend, or could it be written, to apply only in cases where the extra arguments are |
Well we could generalise |
I’m not keen on adding even more syntax for this. It would need to be correctly understood again, and it would be yet another approach to tackle optional arguments. In static functions, arguments are optional if default arguments exist… declare function predicate(
$item as item()*,
$position as xs:integer := ()
) as xs:boolean …so (if we think we need) we should rather choose a similar solution for function items. Next, if we make the first argument(s) mandatory again, we would need to revive some discussions that have been resolved in the past, e.g. on But I definitely agree that the documentation of functions with higher-order function arguments gets more and more arcane, no matter if arguments may be more or less optional. I think we can win a lot by improving the presentation of the arguments of higher-order functions in general. We could start by adding comments for function item arguments. fn:filter(
$input as item()*,
$predicate as function(
item(), (: current item :)
xs:integer (: current position :)
) as xs:boolean
) as item()* This is how the |
It seems a lot more simple to just split this into two separate overloads - as this is done in C# and probably in other languages. What is wrong with having two overloads? From Microsoft's documentation: OverloadsWhere(IEnumerable, Func<TSource,Boolean>) Where(IEnumerable, Func<TSource,Int32,Boolean>) |
We would need a completely different type system. We don't have that kind of polymorphism in the language and it would be an immense effort to introduce it, especially if we were to retain any kind of backwards compatibility. What we could potentially do would be to introduce a general union type so the signature could be
That's essentially equivalent to what I'm proposing. There's a world of difference between having one function that accepts different kinds of arguments, and having two functions where the function despatch algorithm depends on the type of the supplied arguments. You're constantly comparing with C#, which is a strongly typed language. For better or worse, that's not where we are coming from. We are much closer to weakly typed languages like Javascript. |
If we are facing this kind of difficulties, then I'd rather prefer the Thus readability will not suffer, we will continue to be "dynamic" - seems like a win-win situation, doesn't it? |
I think this thread was catalyzed by discussion on |
A quick (and possibly incomplete) search of the function catalog gives for-each filter fold-left fold-right while-do do-until for-each-pair index-where for-each filter fold-left fold-right for-each-pair every index-where some partition |
…and basically every other function that has function arguments. For example, the action of I'm interested what some of you think about my suggestion to focus on our documentation? JavaScript users (and there are lots of them) seem to have no problem with the solution that we're about to take. |
Instead of polluting the definition of well-known and understood functions by adding a new argument that nobody seems to use (I tried hard to find examples of using the index argument!) let us create a separate function that has this extended capabilities - for example we could use a name like: The mere fact that it is so difficult finding examples of actually using this index-argument clearly tells us that this is a separate, much limited functionality. Let us not pollute some of the most-popular and well-known functions in functional programming, making them unrecognizable and difficult to understand. |
As I said at the start of the thread, the main issue is making it clear to the reader what the options are, and we don't necessarily need to change the language to do that: we could go with a documentation solution. Building on Christian's suggestion, something like:
|
Should line 5 be... |
This is still quite unreadable - but we were talking more specifically about the need for having such additional/optional argument for the fold - functions. And there there is absolutely insignificant amount of evidence (concrete code examples of usage) that adding such additional, new argument is really needed in the case of the fold functions. If the new argument would be specified in less that 0.1% of all cases, this clearly means that if (for whatever reason) somebody (whoever they may be) desperately needs it, then they can have a separate new function, say If 80% of the world already knows well the fold functions from languages such as Haskell and C#, why would we want to disguise our fold functions and make them completely unrecognizable to this majority of people? Maybe we intentionally want to repel them from using our languages? |
It could look like that if we decide to add default arguments to dynamically called function items in the future (but there are various implications that would need to be solved). With the current solution in the spec, both arguments are optional, i.e. you can also supply a function like |
For those who want to learn about use cases for folds with index access, two examples in JavaScript: https://stackoverflow.com/questions/35034006/javascript-array-reduce-start-from-index |
I would be happy to go with something that's a bit more type-safe than Javascript, i.e. not allowing arguments to be omitted unless there's something explicit to say they're optional. But I certainly don't accept the argument that something we've adopted from Javascript is going to be unacceptable to our user community. |
Not on this thread we weren't. This thread is about how to indicate in function signatures that some arguments of callback functions are optional. The question of whether to have such optional arguments on the |
True. At the moment, it’s convenient to pass something as simple as let $select := function($test) {
fn:filter($some-data, $test)
}
return $select(true#0) …but if the first parameter of the predicate function of Regarding the syntax, @Arithmeticus’ observation sticks in my mind: Wouldn't it be best and consistent if we introduced default values for function items (as @rhdunn had suggested a long time ago)? Which would certainly be one more hard nut to crack. |
OK, I will be happy just not to add this new argument to the For all other functions, still we need to design their signatures having in mind the importance of readability and understandability. |
These only prove the point that the use of the artificially added new index argument is extremely rare . The blog article proposes a solution for computing the average that uses completely unnecessarily the index argument. This, of course is not needed to find the average. And there is no other, example in all the provided 5 examples that uses the index-argument. As for the StackOverflow question - it was asked 8 years ago, and of the 3 solutions that use the Also, in the last 8 years approximately 2136 Javascript questions were asked on StackOverflow. Only one solution to only one of these tried to use (unnecessarily) the index argument This is 0.05% of all questions, and 0.01% of all solutions. These numbers speak for themselves. To summarize: The provided "evidence" actually tells us that calling the |
Examples and evidence are different things. Conversations are pointless if the mind is fixed. Let’s focus on the actual topic of this issue. |
Still the fact remains that it is difficult to find usage of this parameter and even when found this usage is artificial and unnecessary. As @michaelhkay pointed out, this belongs to the other issue - hope we can keep the folds clean. |
I think this is a good and clear proposal and it is a positive step in solving the issue. I would prefer to have not: but the easier to read (no '%' character): Or even: |
I like the idea of using the
That would resolve the issue of what value should be supplied if the function is called with the lower arity. From an API perspective, should we also make
That's clearer to the user that the second parameter is optional, even if it will never be called with an empty value. |
This doesn't resolve the issue of providing a function with arity 2, when a function with arity 3 is expected. "Forgiving" this results in a flood of unintentional user errors going unnoticed and resulting in difficult to debug runtime errors.
Except that this still requires the call to be:
and not
|
The signature of fn:sum(
$values as xs:anyAtomicType*,
$zero as xs:anyAtomicType? := 0
) as xs:anyAtomicType? …and you can omit the second argument and write |
And what sensible default would there be for the array:filter(
$predicate as function(item()*, xs:integer) as xs:boolean
) as array(*) Wouldn't it be nice if we could provide as the default a 1-argument function? No, because there are so many such 1-argument predicates and each could be handy. Here the real question is not about the default value for a function, or for a function argument. What we really want somehow to do, is specify the default arity of a function, when this arity can vary from one call to another. Even if we specify that one argument is optional, the function still must be of of arity 2, and be able to handle the 2nd argument's (default) value. In many cases when a function of N - 1 arguments is passed N arguments this could be an user error. By masking this error we are doing a bad service to the user, as they will be allowed to execute and will observe unexpected, strange and difficult to explain runtime behavior of their code. |
I would propose
Currently, due/thanks to the function coercion rules, Personally, I believe our major challenge is the improvement of the existing documentation. Think e.g. of: fn:replace(
$value as xs:string?,
$pattern as xs:string,
$replacement as xs:string? := (),
$flags as xs:string? := '',
$action as (function(xs:untypedAtomic, xs:untypedAtomic*) as item()?)? := ()
) as xs:string By just reading the signature, it’s impossible to guess what the parameters of |
I've wondered why function types don't allow named parameters. That's a separate issue, but allowing (optional) named parameters on type signatures would help with documenting the behaviour of the function. |
That’s true. Your issue on supporting optional parameters on dynamic functions is still open as well (#158). I believe to remember that we observed it’s more challenging to realize/formalize this for dynamic function items. |
An integer value cannot be the empty sequence. |
This is even worse! Passing a 0-argument function where a 1-argument function (which is the majority of use cases) or a 2-argument function is expected. This is meaningless and misleading when the main goal of |
All this has already been discussed in the past. |
Whether or not we should closely copy Javascript is a separate topic (which probably we need to discuss separately and reach some conclusion/agreement on), but even if this is so imperative to do, why not copy from the "bigger brother" Typescript, which allows function overloads? Function Overloads in Typescript. Why did they do this? Clearly not for the convenience of the implementors (who, in our case, (said in comments) don't care) but for the Typescript programmers, who need to both easily read the specification of someone else's function and also be able to specify their own-created functions in the most clear and understandable way to future users. In Typescript it is easy to raise a compile-time error when a function is called with 2 arguments, but no overload expects 2 arguments, there are just 2 overloads, expecting either 1 or 3 arguments. I deeply suspect that we are not able to have such level of validation/diagnostics at the moment. |
@dnovatchev The point is that XPath, XQuery, and XSLT function overloading is only based on the arity of the function. Likewise, for types it coerces (matches) values not types. A such, it is not possible to define two functions with the same arity but different parameter types. So they cannot support one taking a Doing so now would break a lot of code and existing implementations, so is not technically feasible in XPath, XQuery, and XSLT. Incidentally, that's the same roadblock that the creators of TypeScript had, hence them creating a separate language to JavaScript! |
Note that in Typescript it’s even discouraged to make parameters of callback (i.e., higher-order) functions optional, as it’s completely valid as well to provide callbacks that accept fewer arguments: |
Well. any function could potentially be used as a callback - then following the above would mean not to specify at all required and optional arguments - that is: all arguments become optional. Down the rabbit hole ... I think the opening comment by @michaelhkay is absolutely correct (bold hi-lighting is mine):
I strongly support the idea that we need to make the language type-safe where this is possible. |
Doing so for ver. 4.0 and later will not break any existing pre-ver. 4.0 code. It is true that having a function reference such as But we could augment the current syntax for a named function reference so that the exact overload becomes part of it, for example something like:
This is the first that comes to mind, maybe there could be even better ways to distinguish between different same-arity overloads. We simply do need to do our job on this, and I believe we can achieve a good and useful result. |
I agree in principle (although I encountered use cases in practice where a 0-arity function like A pragmatic solution would be to simplify the signatures to… fn:xyz(
$input as item()*,
$action as function(*)
) …and define the supported types and supported arities for If we generalized local union types, the following syntax would become legal… fn:xyz(
$input as item()*,
$action as union(
function(item()) as xs:boolean,
function(item(), xs:integer) as xs:boolean
)
) …but pretty verbose. |
Now that we have ChoiceTypes we can resolve this by fn:xyz(
$input as item()*,
$action as function(item()) as xs:boolean | function(item(), xs:integer) as xs:boolean
) |
I think we can close this issue:
|
It was pointed out today that is not obvious, looking at a function signature like
that the second argument of the
$predicate
function is optional.At least in the documentation, it would be useful to capture this in some way. Being "optional" here means that it makes sense, semantically, to supply an arity-1 function, in which case the caller will not supply the second argument.
Perhaps it would also be useful to go beyond documentation, and attach some syntax and semantics to it. Specifically, if the signature of the callback function indicates that the first N arguments are required, then supplying a function item of arity less than N will result in a type error.
The text was updated successfully, but these errors were encountered: