On Tuesday, 14 February 2017 at 20:46:17 UTC, Walter Bright wrote:
On 2/14/2017 8:25 AM, Andrei Alexandrescu 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);

The function name is on the first line.

It could be improved slightly using indentation:

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.

Turns out that this advice is somewhat problematic. When the constraints are too general, it might easily attempt to hijack other calls, leading to conflicts between unrelated functions of the same name.

See https://github.com/dlang/phobos/pull/5149 which introduced a conflict between std.algorithm.copy and std.file.copy for (string, string) arguments.

Still your point is valid, separating different overloads using constraints unnecessarily complicates documentation.

Reply via email to