Range interface for std.container.BinaryHeap #1989
Conversation
|
Could you please add to the documentation that the heap is a standard input range, and add an example too? Thanks! |
|
Done. Also took the liberty of moving the other example's introductory comment out to the unittest documentation. |
|
Could you amend the commit to include the comment |
|
Do I actually need to change a commit, or is modifying the original comment in this thread enough? |
|
@Poita all must be in commit message. |
| { | ||
| int[] a = [4, 1, 3, 2, 16, 9, 10, 14, 8, 7]; | ||
| auto top5 = heapify(a).take(5); | ||
| assert(top5.equal([16, 14, 10, 9, 8])); |
There was a problem hiding this comment.
This should also show/mention that the underlying ranges gets sorted too, by testing: assert(a[0 .. 5].isSorted(). It's not mere "iteration".
There was a problem hiding this comment.
yes please - then let's pull this
There was a problem hiding this comment.
I would mention that if it were true :-) Your assertion fails.
The underlying range is only sorted after complete iteration. This fact is already mentioned in the documentation of BinaryHeap so I see no value in stating it again.
There was a problem hiding this comment.
I would mention that if it were true :-) Your assertion fails.
Right, but because of the backwards sorting. assert(a[0 .. 5].isSorted!"a > b"() should pass.
The underlying range is only sorted after complete iteration.
The parts iterated over so far should be sorted.
So I see no value in stating it again.
I think you can never document things too much, but fine.
There was a problem hiding this comment.
No, it doesn't sort it in reverse either. It sorts it in reverse from the back (a[$-5..$].isSorted).
There was a problem hiding this comment.
No, it doesn't sort it in reverse either.
Ah... See? An example would have helped me with that ;)
Well, actually, that's an unfair comment. I didn't actually the leading documentation.
|
I've rebased and added the "Fixes Issue 12358" message to the commit. |
|
Auto-merge toggled on |
|
Pull updated, auto_merge toggled off |
|
Auto-merge toggled on |
Fixes Issue 12358 See discussion here: dlang#1886
Range interface for std.container.BinaryHeap
See discussion here: #1886
Fixes Issue 12358
https://d.puremagic.com/issues/show_bug.cgi?id=12358