Fix issue 10777 - multiSort should return a SortedRange

[wilzbach]

As I now bumped for the second time into the problem of having a chainable multiSort (=SortedRange), I quickly wrote my own solution. Afterwards I realized that #4061 is already open, there are a couple of minor differences, here

1. a inline, static function instead of MultiPred struct is used.
2. no shrink method is provided (but can be in the future)

So in summary less (possibly controversial) changes.

As always, I am very happy about feedback.

@SzMK - let's join efforts to get this into Phobos?

[dlang-bot]

Fix	Bugzilla	Description
✓	10777	std.algorithm.multiSort to return a std.range.SortedRange

[9il]

Should this be a static fun?

[wilzbach]

thanks for your input @9il - addressed ;-)

[wilzbach]

is there a way how we could write SortedRange here?
The less fun is defined inline.

[9il]

no

[wilzbach]

Another example of the new Travis linting at work :)

There was also a small issue with autotester, that should be fixed now.

"What would be other use cases for MultiPred" (from #4061)

Please not that the difference to the stalled #4061 is that we just use a static, pred function which is passed to assumeSorted, so this is hidden from the user and if wanted we can still extend add a MultiPred struct in the future if wanted (?).

That being said 1) I would like to see MultiPred too and 2) this should be ready to go. The change for the user is that instead of void a SortedRange will be returned. If so, it will be added to the changelog once #4228 is there.

Ping @MartinNowak.

[MartinNowak]

Same comment as on #4061 applies, converting a multi-sort predicate to a sort predicate should be seen as an intermediate step towards a SortedRange w/ multi-sort support, b/c it doesn't make use of the performance benefits (e.g. lowerBound makes M * log(N) comparisions for M predicates).

[MartinNowak]

How about with its predicates converted to an equivalent single predicate.?

[MartinNowak]

Changelog entry missing.

[wilzbach]

Changelog entry missing.

Added (could you please have a look at #4228)?

b/c it doesn't make use of the performance benefits (e.g. lowerBound makes M * log(N) comparisions for M predicates).

Have you looked at the implementation of predFun? It will break as soon as either lessFun(a,b) or lessFun(b, a) is true. Is there really a better way to do this?

All other comments were addressed - and I added a small fix (multiSort didn't work for a single predicate ...)

[wilzbach]

@MartinNowak I am too lazy to rebase all the time, so I removed the changelog entry again. Here is the message:

   $(LI $(XREF algorithm, sorting, multiSort) returns now a
       $(XREF range, SortedRange) instead of `void`)

edit: btw I thought that if it's linked to an issue, you can generate the log entry automatically?

[MartinNowak]

edit: btw I thought that if it's linked to an issue, you can generate the log entry automatically?

Yes, you'll get a Bugzilla fixed entry for references in a commit message. You can recognize that by the check mark in the dlang-bot comment.
#4235 (comment)

[MartinNowak]

Have you looked at the implementation of predFun? It will break as soon as either lessFun(a,b) or lessFun(b, a) is true. Is there really a better way to do this?

Ah great, that's as good as it can get.

[MartinNowak]

Auto-merge toggled on

[CyberShadow]

This pull request introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=16179