syntax sugar: std.path::buildPath instead of from!"std.path".buildPath

[H. S. Teoh via Digitalmars-d]

On Tue, Feb 14, 2017 at 12:46:17PM -0800, Walter Bright via Digitalmars-d wrote:
[...]
> Range remove
>     (SwapStrategy s = SwapStrategy.stable, Range, Offset...)
>     (Range range, Offset offset)
>     if (s != SwapStrategy.stable
>         && isBidirectionalRange!Range
>         && hasLvalueElements!Range
>         && hasLength!Range
>         && Offset.length >= 1);
> 
> But there's another issue here. remove() has other overloads:
> 
> Range remove
>     (SwapStrategy s = SwapStrategy.stable, Range, Offset...)
>     (Range range, Offset offset)
>     if (s == SwapStrategy.stable
>         && isBidirectionalRange!Range
>         && hasLvalueElements!Range
>         && Offset.length >= 1)
> 
> Range remove(alias pred, SwapStrategy s = SwapStrategy.stable, Range)
>     (Range range)
>     if (isBidirectionalRange!Range
>         && hasLvalueElements!Range)
> 
> Two constraints are common to all three, those are the only ones that
> actually need to be in the constraint. The others can go in the body
> under `static if`, as the user need not be concerned with them.
> 
> This is a general issue in Phobos that too many constraints are
> user-facing when they don't need to be.

+1.  I've raised this point many times, but didn't seem to generate much
response.  Basically, there are two more-or-less equivalent ways of
having multiple implementations of a function based on its arguments:
(1) via signature constraints, and (2) via static if's inside the
function (or template) body.

>From an implementor's POV, (1) is more desirable, because it's easy to
add a new overload when you need to extend the function to handle new
types. However, from a user's POV, (2) is much more friendly -- one only
needs to look at the old overloads of std.conv.toImpl to see why: it was
an obtuse mass of only slightly different sig constraints over a
ridiculous number of overloads, each intended to work with very specific
argument combinations, all of which are implementation details the user
need not know about.

Thankfully, the docs for std.conv.to have been greatly improved since
the last time I had to work with the code -- everything is now
consolidated under a single template function std.conv.to, and the
implementation details are hidden behind module-private overloads. This
is the way it should be.

I argue that the same pattern should be applied to (most of the) other
overloaded Phobos functions as well, such as remove() shown above. There
really should only be *one* remove() function with the two constraints
Walter indicated, and the current overloads should either be refactored
under static if's inside the function body, or else renamed and made
into module-private implementation functions called by remove().
Multiple user-facing overloads really only should be used where each
overload represents a *conceptually-different* aspect of the function
(which should be relatively rare).

In general, if a template function (or set of overloads) conceptually
does the same thing, it should be made into a *single* function. If the
implementation differs, that's the problem of the Phobos maintainers,
and should be handled as module-private symbols forwarded to by the
single user-facing function.

Another bonus to this approach is that it allows us to customize nicer
error messages via static assert if none of the (internal) overloads
match the passed argument types. E.g., if the user passes a swappable
bidirectional range to remove() but for whatever reason the arguments
fail to match any of its implementations, then instead of spewing out
pages of inscrutable encrypted Klingon (argument types XYZ do not match
any overloads of remove(), candidates are: [... snip 5 pages of
unreadable function signatures...]), we can, say, do something like:

	static if (...)
		return removeImplA(args);
	else static if (...)
		return removeImplB(args);
	...
	else
		static assert("Arguments to remove() must satisfy conditions X, Y, Z [insert explanation here]");

That's a 1-line error message that tells the user exactly what went
wrong, instead of 5 pages of 10-line function signatures that the user
must parse and then mentally evaluate each Boolean condition for, in
order to deduce exactly why the passed arguments didn't work.


> A further improvement in the documentation would be to add links to
> isBidirectionalRange and hasLvalueElements.

This is a good idea. +1.


T

-- 
"I'm running Windows '98." "Yes." "My computer isn't working now." "Yes, you already said that." -- User-Friendly