Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Improvements to RedBlackTree #10

Merged
2 commits merged into from

3 participants

@jmdavis
Collaborator

Here are some improvements to RedBlackTree as discussed in the newsgroup. It should improve the remove situation and deals with most everything listed in http://d.puremagic.com/issues/show_bug.cgi?id=5451 . I also made it a class, since we're planning to change all of the containers to std.container, and making it a class fixes the default construction problem that it had.

Steven should probably have a look at this before it gets merged in, though I have no idea when he'll get around to that, since he hasn't gotten around to figuring out git yet. So, if multiple other folks look at it and think that it's good though, then we should probably just merge it in and let Steven worry about making whatever changes he thinks are appropriate later. Certainly, these changes make RedBlackTree more usable.

@jmdavis jmdavis Improvements to RedBlackTree.
1. It's now a class. The only constructor is now the default
constructor (since templatizing the constructors does not appear to
currently work).

2. It now has a helper function - redBlackTree - for creating it.

3. It now has a length property.

4. It now has a version of remove which takes a Take!Range.

5. It now has removeKey to allow you to remove one or more elements by value from it.
aca1a2d
@andralex

This should be if (is(typeof(binaryFun!less(T.init, T.init))))

@andralex

probably version(xxx) unittest would be more idiomatic.

Collaborator

There is a very good reason I did this in dcollections. The reason is because doUnittest is only true if the element type is integral. If the doUnittest enum is not used, then simply instantiating a RedBlackTree!string fails because the unit tests are not valid.

If there are better ways to do this, I'd love to change this. I think the way this is done is rather ugly, but it does work.

Collaborator

Apparently, version can only be used at the module level, so I'm leaving it as is. For the trick to work where it instantiates the unit tests for each instantiation of the template which meets certain requirements (isIntegral!T in this case), you have to use static if rather than version.

Owner

I understand. So the flag facilitiates putting unittests next to the methods they are testing. I guess I'd prefer hoisting them out of the class, but I reckon it's good to have the unittest right there.

Collaborator

For reference, here is some ideas I had when I discovered this issue. I believe for unit tests to be maintainable, they need to be next to the function being tested. Otherwise, nobody is going to go to the end of the class and find the unittest to update. If the "unittest to ddoc example" thing ever gets working, then putting unit tests near the function is a requirement.

http://www.digitalmars.com/webnews/newsgroups.php?art_group=digitalmars.D.learn&article_id=19733

Collaborator

I totally agree that it's better to have unit tests next to the functions that they're testing, but it doesn't work as well in templated types. And in the general case, I wouldn't expect it to work with the "unittest to ddoc example" thing, because it's likely to be hard to have a working example in the template like that. You have to pull tricks like RedBlackTree is doing and static if unittest blocks based on the template arguments. And if you have to use version(D_Ddoc) it's even worse, since then you can't put a functioning unittest block next to the function. So, there will be cases where it's just not going to work to use the "unittest to ddoc example" thing no matter how desirable it might be.

Regardless, in the general case, it's definitely more maintainable to put the unit tests next to the code that they test. It's a major irritation for me that I had to put the unit tests for the templated types like the interval types after the type definition instead of next to the functions that they tested, and it definitely makes maintaining that harder. So, any time that we can reasonably put the unit tests next to what they test, it's definitely the way to go.

@andralex
Collaborator

If you wish. I felt dup was a function, not a property. I know the precedent in arrays is as a property, but dup is not a simple accessor. Requiring the parens makes it seem more like a 'function'.

Collaborator

I really don't think that dup is a property (just like I don't think that save or ranges is really a property), but since it's a property on arrays, it seems better to just make it match and make it a property.

Owner

I think we should do what arrays do instead of gratuitously choosing something different. I'd hate to require the user to remember to add parens for containers but not for arrays. For that reason documented intently c.dup without parens in the table in std.container.

@andralex

One problem is that each instantiation of RedBlackTree will generate and run another unittest. So for parameterized structs and classes it's best to move unittests outside, unless they depend on the template parameters (which doesn't seem to be the case here).

Collaborator

I believe that that was Steven's intent. I have no problem with moving them out however. The benefit in testing specifically integral types like it's doing is limited.

Collaborator

It only instantiates and runs another unit test if the element type is integral. This is the whole purpose of doUnittest.

I highly recommend to leave things the way they are, I found about 5 dmd bugs while doing unit tests simply because it was extremely easy to add more unit tests for another integer type (I test all 8).

It might be possible to do more types, but I'd rather figure out how to change the current unit tests so they work with the new type also than move the unit tests outside.

Collaborator

The problem is that then you can't test other comparison functions very well. And honestly, I would consider integral types simple enough that testing all of them is of limited benefit. At most, you find some errors related to narrowing conversions - at least as far as finding bugs in RedBlackTree goes. Maybe there are dmd bugs that it would help find. I have no idea what types of bugs you found before, so I can't really comment on that. But assuming that dmd works correctly, I see limited benefit.

Collaborator

there were a few bugs in containers that I found in dcollections that didn't show up until I was using shorts and ushorts, and the ones I found in dmd were related to AA's with keys smaller than integers (would not have been found here, AA's aren't used)

You should be able to add unit tests for other types. For example, you could also test strings. What needs to be done is to alter the create function so it takes ints and use std.conv.to to convert to Elem. You'd also need to change all casts to to!Elem.

You can always add unit tests outside the class if you want to test other types. But to be able to test comprehensive unit tests across a type by just instantiating the class is very slick I think.

Collaborator

I agree that declaring unittest blocks inside of templatized types can be very useful, but I've generally found that too many tests don't quite work that way, so I rarely do it. Using static if or a version based on static if as you've done does ameloriate the problem, but it doesn't entirely fix it.

@andralex

I'm pretty sure you can do here without precomputing len. Use the value of "i" in the loop below at the termination of the loop. Of course you'd need to hoist "i"'s definition outside the for-init-statement.

@andralex

Would be great to avoid duplication here by e.g. forwarding from the first function to the second. To force an array into a non-array range, use e.g. filter!"true"(elems).

Collaborator

Ah. There we go. The algorithms that I thought of trying to turn the array into another range type all ended up keeping it as an array.

@andralex

Oh wait, here you convert to array anyway. So you may decide to forward the other way around.

Collaborator

Good point.

Collaborator

I was about to comment that I don't like how array allocates here, but I realize why you did it. I can't say I would have made the same choice, but I find no fault with it.

Note, you can specialize removeKey for RedBlackTrees that do not allow duplicates (there will be only one element to remove, no need to use equalRange).

Actually, I question that you remove all elements with the same key, what if you only wanted to remove one of them?

In dcollections, I simply ignored the possibility, and if someone wished to add or remove data to/from itself, I only check the direct cases.

@andralex
Owner

Nice work. A few nits.

Collaborator

Yeah. Some good points there, though some of them were there in the original code and not my doing (though they're still worth looking at).

Owner

Understood. I'm thinking since you're touching the file anyway it's worth making some opportunistic improvements too.

@schveiguy
Collaborator

Probably can remove the comment about Bug 2810, I think that was just fixed. However, I haven't tested yet...

@schveiguy
Collaborator

Why change this from the ctor that takes elements to using the default ctor and stableInsert?
Not that it's a big deal, it just seems like an odd change.

Collaborator

The problem is the inability to have properly templatized class constructors. So, I just simplified it so that you have just the default constructor and added the redBlackTree helper function to essentially take the place of the constructor (with the added benefit of not necessarily having to give the element type). I suppose that we could have this(E[] elems...), but the templatized version that takes implicit conversions into account and whatnot won't work. But it just seems simpler to me to push off construction to redBlackTree in all cases where you don't want default construction rather than having it work with the constructor in some cases but be forced to use redBlackTree in others.

Collaborator

Yes, I wrote this before I saw you disabled the template ctors.

this(E[] elems...) should do implicit conversions without issue, as long as you are passing elements. What you won't be able to do is instantiate with a range type.

However, it should be fully backwards compatible to later remove the this(E[] elems...) and add the template version, so I'd rather see it added now. To have new RedBlackTree!int(1,2,3,4,5) not work seems unintuitive.

@schveiguy
Collaborator

It is probably not necessary to add length checks to so many unit tests.

@schveiguy
Collaborator

BTW, I like the scope(success) usage to alter length, I'm going to steal that idea for dcollections too ;)

@schveiguy
Collaborator

I think this can be done simpler:

auto b = r.original._begin;
while(!r.empty) r.popFront(); // move take range to its last element
auto e = r.original._begin;
while(b != e) {b = b.remove(_end); --_length;}
return Range(e, _end);

(BTW, a lot of the above has leading underscores, but github seems to think that's markup for italics, sorry)

Also, I would expect the removal of an empty take range to return the range that starts from wherever the take range started. The code above does that, your code does not.

for example:

auto ts = create(1,2,3,4,5);
auto r = ts[];
r.popFront();
auto r2 = ts.remove(take(r, 0));
assert(std.algorithm.equal(r2, [2,3,4,5]));

Collaborator

I'll adjust it then.

@schveiguy
Collaborator

OK, I see now why you stopped using the ctor above.

However, as an intermediate step, let's just create a ctor like:

this(Elems[] elems...)

instead of having to use create (which wasn't a template anyways). When template ctors become possible, changing this to a template should be a completely backwards compatible change.

@jmdavis
Collaborator

@schveiguy I could probably remove length from some of the unit tests, but given all of the various places that length has to be changed, it seemed a lot safer to just be thorough about it.

@schveiguy
Collaborator

Can the above two be merged into:

auto redBlackTree(alias less, bool allowDuplicates=false, E)(E[] elems...)

or does that not work?

Collaborator

The reason that I have so many versions or redBlackTree is that you can give any combination of the template parameters and let the others be their default values rather than being forced to give some of the parameters, because the one you want to set is later in the list. It's possible that I declared one more than is needed, but I'd have to mess around with it to be sure.

Collaborator

Okay. It turns out that they can't be merge. You get conflicts if you try. On top of that, there's some weird bug where the last one doesn't work, because RedBlackTree barfs on less with an error like this:

std/container.d(4136): Error: this for _end needs to be type RedBlackTree not type std.container.RedBlackTree!(int,"a > b",true).RedBlackTree

However, passing binaryFun!less to the template instantiation of RedBlackTree fixes the problem.

Collaborator

OK, having the extra templates doesn't hurt. It really was a trivial nitpick.

Thanks for trying though.

@schveiguy
Collaborator

Overall: looks good, need to fix that remove+Take function (see comment about what should be returned when removing an empty range).

@jmdavis jmdavis Changes to RedBlackTree per suggestions on pull request #10 of D-Prog…
…ramming-Language/Phobos.

I think that I took all of the suggestions into account, so it should be
improved per those suggestions. I also beefed up the unit tests a bit in
order to take different less and allowDuplicate values into account
(previously, it was only testing the defaults for those template
parameters).
858865a
@jmdavis
Collaborator

I've made the changes as suggested and improved the unit tests a bit.

@schveiguy
Collaborator

I like how you are expanding the unit tests to test the duplicate version and altering the ordering.

I think this should be:

else static if(less == "a > b")

because there is a possibility some other instantiation would be created with an integral type, and the unittest shouldn't assume the ordering.

@schveiguy
Collaborator

Nice, I like this test jig. Also stealing for dcollections ;)

@schveiguy
Collaborator

I think this commit looks good enough. The "else static if" comment isn't critical to the patch, so if we want to accept this as is, I approve.

@andralex
Owner

I tried to merge this, by following the instructions. However, unittests fail:

std/datetime.d(28538): Error: undefined identifier tzname, did you mean function name?

What's going on?

@jmdavis
Collaborator

Update druntime. The declaration of tzname got moved to drutime where it belongs.

@andralex
Owner

Done.

@kyllingstad kyllingstad referenced this pull request from a commit in kyllingstad/phobos
@jmdavis jmdavis Changes to RedBlackTree per suggestions on pull request #10 of D-Prog…
…ramming-Language/Phobos.

I think that I took all of the suggestions into account, so it should be
improved per those suggestions. I also beefed up the unit tests a bit in
order to take different less and allowDuplicate values into account
(previously, it was only testing the defaults for those template
parameters).
c6874cb
@klickverbot klickverbot referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@klickverbot klickverbot referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
99e1208
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
5367827
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
179e57c
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
9cfaa7d
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
3c7da8d
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
95f9901
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
6ef364a
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
6cfdf48
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
c678640
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
7c64303
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
d24f031
@mleise mleise referenced this pull request from a commit in mleise/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
33271a9
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
ac9c8b3
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
d6d46d4
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
0c093fe
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
f0aac4b
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
0c80d7d
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
2d7821e
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
8bdd5a7
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
d12bbd2
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
d393a37
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
1c73233
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
9de481a
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
0498998
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
7b82a45
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
836fab7
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
d58dec4
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
1d6b84e
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
4e51092
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner thread safe and concur works comments for free standing log functions
fixes #10
f648be2
@burner burner referenced this pull request from a commit in burner/phobos
@burner burner std.logger
some more docu fixes

fatel -> fatal

changed the way of calling

some rework

some better comments and win32.mak fix

more documentation

trying to figure out why win32 fails

test before you commit

another try to work around windows files and processes

maybe this time

maybe this merges

update 1 mostly soenke

MultiLogger and Logger have names

more docu

unittest fix

some better comments and win32.mak fix

Conflicts:
	std/logger.d

antoher doc fix

less code dup and more log functions

docs are now generated

some more fixing

speedup

some threading

forgot some

better docu gen and more testing

two new LogLevel, most functions are now at least @trusted

Tracer functionality

fatal delegate

some phobos style fixes

another bug bites the dust

version disable enhancements

default global LogLevel set to LogLevel.all

logLevel compare bugfix

delete of dead code

tab to whitespace

bugfixes, reduandency removal, and docu

multilogger was logging to all it childs without checking there LogLevel in
relation to the LogLevel of the LoggerPayload.

Some constructors where redundant.

more examples and more documentation

some fixes

NullLogger and moduleName

wrong doc

I splitted the multi logger implementations out

loglevelF to loglevelf in order to make phobos style think writefln

document building

win makefile fixes

some optimizations thanks to @elendel-
some whitespace

some updates from the github logger project

* stdio- and filelogger now use a templatelogger base to reduce code

makefile fixes

makefile

fixed the unittest

made sure the stdiologger is set up at the end

some comment fixes

finding the filelogger segfault

closed another file

a lookup fix

darwin unittest fail output

more diagnostics

* more documentation for the templatelogger and multilogger
* it breaks the log function api
 * log and logf now behave as their write and writef counterparts
 * for logging with an explicit LogLevel call logl loglf
 * for logging with an explicit condition call logc logcf
 * for logging with an explicit LogLevel and explicit condition call
 * loglc loglcf
 * the log function with an explicit LogLevel like info, warning, ...
 * can be extended into c, f or cf and therefore require a condition
 * and/or are printf functions

something is still interesting

rebase and lazy Args

saver file handling

whitespace

some updates

tracer is gone and more doc updates

codegen rework to allow log function comments

thread safe and concur works comments for free standing log functions

fixes #10

* free log function doco
* StdIOLogger prints now threadsafe
* concurrentcy hang fix

more docu

old file

even more doc

typo

* more better doc
* makefile fix

another fix

more unittests nearl 100% everywhere

another test

no more mixins more doc

win64 please pass

fixes from the review and voting

hope this fixes it

more comments

more docs more tests

more docu

more doc and fixes from jacob-carlborg

LogEntry.logger reference

more fixes

fileptr is gone

logger move to experimental/logger

type in win32/64 makefile

win makefile

nogc and threading

makefile fixes

some log calls take line and file as parameter
04aa499
@andralex andralex referenced this pull request from a commit in andralex/phobos
@burner burner std.logger
some more docu fixes

fatel -> fatal

changed the way of calling

some rework

some better comments and win32.mak fix

more documentation

trying to figure out why win32 fails

test before you commit

another try to work around windows files and processes

maybe this time

maybe this merges

update 1 mostly soenke

MultiLogger and Logger have names

more docu

unittest fix

some better comments and win32.mak fix

Conflicts:
	std/logger.d

antoher doc fix

less code dup and more log functions

docs are now generated

some more fixing

speedup

some threading

forgot some

better docu gen and more testing

two new LogLevel, most functions are now at least @trusted

Tracer functionality

fatal delegate

some phobos style fixes

another bug bites the dust

version disable enhancements

default global LogLevel set to LogLevel.all

logLevel compare bugfix

delete of dead code

tab to whitespace

bugfixes, reduandency removal, and docu

multilogger was logging to all it childs without checking there LogLevel in
relation to the LogLevel of the LoggerPayload.

Some constructors where redundant.

more examples and more documentation

some fixes

NullLogger and moduleName

wrong doc

I splitted the multi logger implementations out

loglevelF to loglevelf in order to make phobos style think writefln

document building

win makefile fixes

some optimizations thanks to @elendel-
some whitespace

some updates from the github logger project

* stdio- and filelogger now use a templatelogger base to reduce code

makefile fixes

makefile

fixed the unittest

made sure the stdiologger is set up at the end

some comment fixes

finding the filelogger segfault

closed another file

a lookup fix

darwin unittest fail output

more diagnostics

* more documentation for the templatelogger and multilogger
* it breaks the log function api
 * log and logf now behave as their write and writef counterparts
 * for logging with an explicit LogLevel call logl loglf
 * for logging with an explicit condition call logc logcf
 * for logging with an explicit LogLevel and explicit condition call
 * loglc loglcf
 * the log function with an explicit LogLevel like info, warning, ...
 * can be extended into c, f or cf and therefore require a condition
 * and/or are printf functions

something is still interesting

rebase and lazy Args

saver file handling

whitespace

some updates

tracer is gone and more doc updates

codegen rework to allow log function comments

thread safe and concur works comments for free standing log functions

fixes #10

* free log function doco
* StdIOLogger prints now threadsafe
* concurrentcy hang fix

more docu

old file

even more doc

typo

* more better doc
* makefile fix

another fix

more unittests nearl 100% everywhere

another test

no more mixins more doc

win64 please pass

fixes from the review and voting

hope this fixes it

more comments

more docs more tests

more docu

more doc and fixes from jacob-carlborg

LogEntry.logger reference

more fixes

fileptr is gone

logger move to experimental/logger

type in win32/64 makefile

win makefile

nogc and threading

makefile fixes

some log calls take line and file as parameter
acb31a3
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 23, 2011
  1. @jmdavis

    Improvements to RedBlackTree.

    jmdavis authored
    1. It's now a class. The only constructor is now the default
    constructor (since templatizing the constructors does not appear to
    currently work).
    
    2. It now has a helper function - redBlackTree - for creating it.
    
    3. It now has a length property.
    
    4. It now has a version of remove which takes a Take!Range.
    
    5. It now has removeKey to allow you to remove one or more elements by value from it.
Commits on Feb 24, 2011
  1. @jmdavis

    Changes to RedBlackTree per suggestions on pull request #10 of D-Prog…

    jmdavis authored
    …ramming-Language/Phobos.
    
    I think that I took all of the suggestions into account, so it should be
    improved per those suggestions. I also beefed up the unit tests a bit in
    order to take different less and allowDuplicate values into account
    (previously, it was only testing the defaults for those template
    parameters).
This page is out of date. Refresh to see the latest.
Showing with 417 additions and 60 deletions.
  1. +417 −60 std/container.d
View
477 std/container.d
@@ -4105,13 +4105,10 @@ struct RBNode(V)
* ignored on insertion. If duplicates are allowed, then new elements are
* inserted after all existing duplicate elements.
*/
-struct RedBlackTree(T, alias less = "a < b", bool allowDuplicates = false)
-if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
+class RedBlackTree(T, alias less = "a < b", bool allowDuplicates = false)
+ if(is(typeof(binaryFun!less(T.init, T.init))))
{
- static if(is(typeof(less) == string))
- alias binaryFun!(less) _less;
- else
- alias less _less;
+ alias binaryFun!less _less;
// BUG: this must come first in the struct due to issue 2810
@@ -4122,7 +4119,20 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
{
Node result;
static if(!allowDuplicates)
+ {
bool added = true;
+ scope(success)
+ {
+ if(added)
+ ++_length;
+ }
+ }
+ else
+ {
+ scope(success)
+ ++_length;
+ }
+
if(!_end.left)
{
_end.left = result = allocate(n);
@@ -4204,11 +4214,6 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
}
return false;
}
-
- private static RedBlackTree create(Elem[] elems...)
- {
- return RedBlackTree(elems);
- }
}
else
{
@@ -4223,10 +4228,12 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
// used for convenience
private alias RBNode!Elem.Node Node;
- private Node _end;
+ private Node _end;
+ private size_t _length;
private void _setup()
{
+ assert(!_end); //Make sure that _setup isn't run more than once.
_end = allocate();
}
@@ -4311,10 +4318,17 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1, 2, 3, 4, 5);
+ auto ts = new RedBlackTree(1, 2, 3, 4, 5);
+ assert(ts.length == 5);
auto r = ts[];
- assert(std.algorithm.equal(r, cast(T[])[1, 2, 3, 4, 5]));
- assert(r.front == 1);
+
+ static if(less == "a < b")
+ auto vals = [1, 2, 3, 4, 5];
+ else
+ auto vals = [5, 4, 3, 2, 1];
+
+ assert(std.algorithm.equal(r, vals));
+ assert(r.front == vals.front);
assert(r.back != r.front);
auto oldfront = r.front;
auto oldback = r.back;
@@ -4323,6 +4337,7 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
assert(r.front != r.back);
assert(r.front != oldfront);
assert(r.back != oldback);
+ assert(ts.length == 5);
}
// find a node based on an element value
@@ -4372,27 +4387,37 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
return _end.left is null;
}
+ /++
+ Returns the number of elements in the container.
+
+ Complexity: $(BIGOH 1).
+ +/
+ @property size_t length()
+ {
+ return _length;
+ }
+
/**
* Duplicate this container. The resulting container contains a shallow
* copy of the elements.
*
* Complexity: $(BIGOH n)
*/
- RedBlackTree dup()
+ @property RedBlackTree dup()
{
- RedBlackTree result;
- result._setup();
- result._end = _end.dup();
- return result;
+ return new RedBlackTree(_end.dup(), _length);
}
static if(doUnittest) unittest
{
- auto ts = create(1, 2, 3, 4, 5);
- auto ts2 = ts.dup();
+ auto ts = new RedBlackTree(1, 2, 3, 4, 5);
+ assert(ts.length == 5);
+ auto ts2 = ts.dup;
+ assert(ts2.length == 5);
assert(std.algorithm.equal(ts[], ts2[]));
ts2.insert(cast(Elem)6);
assert(!std.algorithm.equal(ts[], ts2[]));
+ assert(ts.length == 5 && ts2.length == 6);
}
/**
@@ -4425,11 +4450,12 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
return _end.prev.value;
}
- /**
- * Check to see if an element exists in the container
- *
- * Complexity: $(BIGOH log(n))
- */
+ /++
+ $(D in) operator. Check to see if the given element exists in the
+ container.
+
+ Complexity: $(BIGOH log(n))
+ +/
bool opBinaryRight(string op)(Elem e) if (op == "in")
{
return _find(e) !is null;
@@ -4437,19 +4463,28 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1, 2, 3, 4, 5);
+ auto ts = new RedBlackTree(1, 2, 3, 4, 5);
assert(cast(Elem)3 in ts);
assert(cast(Elem)6 !in ts);
}
/**
- * Clear the container of all elements
+ * Removes all elements from the container.
*
* Complexity: $(BIGOH 1)
*/
void clear()
{
_end.left = null;
+ _length = 0;
+ }
+
+ static if(doUnittest) unittest
+ {
+ auto ts = new RedBlackTree(1,2,3,4,5);
+ assert(ts.length == 5);
+ ts.clear();
+ assert(ts.empty && ts.length == 0);
}
/**
@@ -4504,10 +4539,33 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1,2,3,4,5);
- assert(ts.stableInsert(cast(Elem[])[6, 7, 8, 9, 10]) == 5);
- assert(ts.stableInsert(cast(Elem)11) == 1);
- assert(ts.arrayEqual([1,2,3,4,5,6,7,8,9,10,11]));
+ auto ts = new RedBlackTree(2,1,3,4,5,2,5);
+ static if(allowDuplicates)
+ {
+ assert(ts.length == 7);
+ assert(ts.stableInsert(cast(Elem[])[7, 8, 6, 9, 10, 8]) == 6);
+ assert(ts.length == 13);
+ assert(ts.stableInsert(cast(Elem)11) == 1 && ts.length == 14);
+ assert(ts.stableInsert(cast(Elem)7) == 1 && ts.length == 15);
+
+ static if(less == "a < b")
+ assert(ts.arrayEqual([1,2,2,3,4,5,5,6,7,7,8,8,9,10,11]));
+ else
+ assert(ts.arrayEqual([11,10,9,8,8,7,7,6,5,5,4,3,2,2,1]));
+ }
+ else
+ {
+ assert(ts.length == 5);
+ assert(ts.stableInsert(cast(Elem[])[7, 8, 6, 9, 10, 8]) == 5);
+ assert(ts.length == 10);
+ assert(ts.stableInsert(cast(Elem)11) == 1 && ts.length == 11);
+ assert(ts.stableInsert(cast(Elem)7) == 0 && ts.length == 11);
+
+ static if(less == "a < b")
+ assert(ts.arrayEqual([1,2,3,4,5,6,7,8,9,10,11]));
+ else
+ assert(ts.arrayEqual([11,10,9,8,7,6,5,4,3,2,1]));
+ }
}
/**
@@ -4517,6 +4575,8 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
*/
Elem removeAny()
{
+ scope(success)
+ --_length;
auto n = _end.leftmost;
auto result = n.value;
n.remove(_end);
@@ -4527,8 +4587,10 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1,2,3,4,5);
+ auto ts = new RedBlackTree(1,2,3,4,5);
+ assert(ts.length == 5);
auto x = ts.removeAny();
+ assert(ts.length == 4);
Elem[] arr;
foreach(Elem i; 1..6)
if(i != x) arr ~= i;
@@ -4542,6 +4604,8 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
*/
void removeFront()
{
+ scope(success)
+ --_length;
_end.leftmost.remove(_end);
version(RBDoChecks)
check();
@@ -4554,6 +4618,8 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
*/
void removeBack()
{
+ scope(success)
+ --_length;
_end.prev.remove(_end);
version(RBDoChecks)
check();
@@ -4561,19 +4627,29 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1,2,3,4,5);
+ auto ts = new RedBlackTree(1,2,3,4,5);
+ assert(ts.length == 5);
ts.removeBack();
- assert(ts.arrayEqual([1,2,3,4]));
+ assert(ts.length == 4);
+
+ static if(less == "a < b")
+ assert(ts.arrayEqual([1,2,3,4]));
+ else
+ assert(ts.arrayEqual([2,3,4,5]));
+
ts.removeFront();
- assert(ts.arrayEqual([2,3,4]));
+ assert(ts.arrayEqual([2,3,4]) && ts.length == 3);
}
- /**
- * Remove the given range from the container. Returns a range containing
- * all the elements that were after the given range.
- *
- * Complexity: $(BIGOH m * log(n))
- */
+ /++
+ Removes the given range from the container.
+
+ Returns: A range containing all of the elements that were after the
+ given range.
+
+ Complexity: $(BIGOH m * log(n)) (where m is the number of elements in
+ the range)
+ +/
Range remove(Range r)
{
auto b = r._begin;
@@ -4581,6 +4657,7 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
while(b !is e)
{
b = b.remove(_end);
+ --_length;
}
version(RBDoChecks)
check();
@@ -4589,13 +4666,159 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1,2,3,4,5);
+ auto ts = new RedBlackTree(1,2,3,4,5);
+ assert(ts.length == 5);
auto r = ts[];
r.popFront();
r.popBack();
+ assert(ts.length == 5);
auto r2 = ts.remove(r);
+ assert(ts.length == 2);
assert(ts.arrayEqual([1,5]));
- assert(std.algorithm.equal(r2, [5]));
+
+ static if(less == "a < b")
+ assert(std.algorithm.equal(r2, [5]));
+ else
+ assert(std.algorithm.equal(r2, [1]));
+ }
+
+ /++
+ Removes the given $(D Take!Range) from the container
+
+ Returns: A range containing all of the elements that were after the
+ given range.
+
+ Complexity: $(BIGOH m * log(n)) (where m is the number of elements in
+ the range)
+ +/
+ Range remove(Take!Range r)
+ {
+ auto b = r.original._begin;
+
+ while(!r.empty)
+ r.popFront(); // move take range to its last element
+
+ auto e = r.original._begin;
+
+ while(b != e)
+ {
+ b = b.remove(_end);
+ --_length;
+ }
+
+ return Range(e, _end);
+ }
+
+ static if(doUnittest) unittest
+ {
+ auto ts = new RedBlackTree(1,2,3,4,5);
+ auto r = ts[];
+ r.popFront();
+ assert(ts.length == 5);
+ auto r2 = ts.remove(take(r, 0));
+
+ static if(less == "a < b")
+ {
+ assert(std.algorithm.equal(r2, [2,3,4,5]));
+ auto r3 = ts.remove(take(r, 2));
+ assert(ts.arrayEqual([1,4,5]) && ts.length == 3);
+ assert(std.algorithm.equal(r3, [4,5]));
+ }
+ else
+ {
+ assert(std.algorithm.equal(r2, [4,3,2,1]));
+ auto r3 = ts.remove(take(r, 2));
+ assert(ts.arrayEqual([5,2,1]) && ts.length == 3);
+ assert(std.algorithm.equal(r3, [2,1]));
+ }
+ }
+
+ /++
+ Removes elements from the container that are equal to the given values
+ according to the less comparator. One element is removed for each value
+ given which is in the container. If $(D allowDuplicates) is true,
+ duplicates are removed only if duplicate values are given.
+
+ Returns: The number of elements removed.
+
+ Complexity: $(BIGOH m log(n)) (where m is the number of elements to remove)
+
+ Examples:
+--------------------
+auto rbt = redBlackTree!true(0, 1, 1, 1, 4, 5, 7);
+rbt.removeKey(1, 4, 7);
+assert(std.algorithm.equal(rbt[], [0, 1, 1, 5]));
+rbt.removeKey(1, 1, 0);
+assert(std.algorithm.equal(rbt[], [5]));
+--------------------
+ +/
+ size_t removeKey(U)(U[] elems...)
+ if(isImplicitlyConvertible!(U, Elem))
+ {
+ immutable lenBefore = length;
+
+ foreach(e; elems)
+ {
+ auto beg = _firstGreaterEqual(e);
+ if(beg is _end || _less(e, beg.value))
+ // no values are equal
+ continue;
+ beg.remove(_end);
+ --_length;
+ }
+
+ return lenBefore - length;
+ }
+
+ /++ Ditto +/
+ size_t removeKey(Stuff)(Stuff stuff)
+ if(isInputRange!Stuff &&
+ isImplicitlyConvertible!(ElementType!Stuff, Elem) &&
+ !is(Stuff == Elem[]))
+ {
+ //We use array in case stuff is a Range from this RedBlackTree - either
+ //directly or indirectly.
+ return removeKey(array(stuff));
+ }
+
+ static if(doUnittest) unittest
+ {
+ auto rbt = new RedBlackTree(5, 4, 3, 7, 2, 1, 7, 6, 2, 19, 45);
+
+ static if(allowDuplicates)
+ {
+ assert(rbt.length == 11);
+ assert(rbt.removeKey(cast(Elem)4) == 1 && rbt.length == 10);
+ assert(rbt.arrayEqual([1,2,2,3,5,6,7,7,19,45]) && rbt.length == 10);
+
+ assert(rbt.removeKey(cast(Elem)6, cast(Elem)2, cast(Elem)1) == 3);
+ assert(rbt.arrayEqual([2,3,5,7,7,19,45]) && rbt.length == 7);
+
+ assert(rbt.removeKey(cast(Elem)(42)) == 0 && rbt.length == 7);
+ assert(rbt.removeKey(take(rbt[], 3)) == 3 && rbt.length == 4);
+
+ static if(less == "a < b")
+ assert(std.algorithm.equal(rbt[], [7,7,19,45]));
+ else
+ assert(std.algorithm.equal(rbt[], [7,5,3,2]));
+ }
+ else
+ {
+ assert(rbt.length == 9);
+ assert(rbt.removeKey(cast(Elem)4) == 1 && rbt.length == 8);
+ assert(rbt.arrayEqual([1,2,3,5,6,7,19,45]));
+
+ assert(rbt.removeKey(cast(Elem)6, cast(Elem)2, cast(Elem)1) == 3);
+ assert(rbt.arrayEqual([3,5,7,19,45]) && rbt.length == 5);
+
+ assert(rbt.removeKey(cast(Elem)(42)) == 0 && rbt.length == 5);
+ assert(rbt.removeKey(take(rbt[], 3)) == 3 && rbt.length == 2);
+
+ static if(less == "a < b")
+ assert(std.algorithm.equal(rbt[], [19,45]));
+ else
+ assert(std.algorithm.equal(rbt[], [5,3]));
+ }
}
// find the first node where the value is > e
@@ -4685,18 +4908,28 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
static if(doUnittest) unittest
{
- auto ts = create(1, 2, 3, 4, 5);
- auto r1 = ts.lowerBound(3);
- assert(std.algorithm.equal(r1, [1,2]));
- auto r2 = ts.upperBound(3);
- assert(std.algorithm.equal(r2, [4,5]));
- auto r3 = ts.equalRange(3);
- assert(std.algorithm.equal(r3, [3]));
+ auto ts = new RedBlackTree(1, 2, 3, 4, 5);
+ auto rl = ts.lowerBound(3);
+ auto ru = ts.upperBound(3);
+ auto re = ts.equalRange(3);
+
+ static if(less == "a < b")
+ {
+ assert(std.algorithm.equal(rl, [1,2]));
+ assert(std.algorithm.equal(ru, [4,5]));
+ }
+ else
+ {
+ assert(std.algorithm.equal(rl, [5,4]));
+ assert(std.algorithm.equal(ru, [2,1]));
+ }
+
+ assert(std.algorithm.equal(re, [3]));
}
version(RBDoChecks)
{
- /**
+ /*
* Print the tree. This prints a sideways view of the tree in ASCII form,
* with the number of indentations representing the level of the nodes.
* It does not print values, only the tree structure and color of nodes.
@@ -4721,7 +4954,7 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
writeln();
}
- /**
+ /*
* Check the tree for validity. This is called after every add or remove.
* This should only be enabled to debug the implementation of the RB Tree.
*/
@@ -4777,6 +5010,11 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
}
}
+ /+
+ For the moment, using templatized contstructors doesn't seem to work
+ very well (likely due to bug# 436 and/or bug# 1528). The redBlackTree
+ helper function seems to do the job well enough though.
+
/**
* Constructor. Pass in an array of elements, or individual elements to
* initialize the tree with.
@@ -4795,17 +5033,136 @@ if (is(typeof(less(T.init, T.init)) == bool) || is(typeof(less) == string))
_setup();
stableInsert(stuff);
}
+ +/
+
+ /++ +/
+ this()
+ {
+ _setup();
+ }
+
+ /++
+ Constructor. Pass in an array of elements, or individual elements to
+ initialize the tree with.
+ +/
+ this(Elem[] elems...)
+ {
+ _setup();
+ stableInsert(elems);
+ }
+
+ private this(Node end, size_t length)
+ {
+ _end = end;
+ _length = length;
+ }
+}
+
+//Verify Example for removeKey.
+unittest
+{
+ auto rbt = redBlackTree!true(0, 1, 1, 1, 4, 5, 7);
+ rbt.removeKey(1, 4, 7);
+ assert(std.algorithm.equal(rbt[], [0, 1, 1, 5]));
+ rbt.removeKey(1, 1, 0);
+ assert(std.algorithm.equal(rbt[], [5]));
+}
+
+unittest
+{
+ void test(T)()
+ {
+ auto rt1 = new RedBlackTree!(T, "a < b", false)();
+ auto rt2 = new RedBlackTree!(T, "a < b", true)();
+ auto rt3 = new RedBlackTree!(T, "a > b", false)();
+ auto rt4 = new RedBlackTree!(T, "a > b", true)();
+ }
+
+ test!long();
+ test!ulong();
+ test!int();
+ test!uint();
+ test!short();
+ test!ushort();
+ test!byte();
+ test!byte();
+}
+
+/++
+ Convenience function for creating a $(D RedBlackTree!E) from a list of
+ values.
+
+ Examples:
+--------------------
+auto rbt1 = redBlackTree(0, 1, 5, 7);
+auto rbt2 = redBlackTree!string("hello", "world");
+auto rbt3 = redBlackTree!true(0, 1, 5, 7, 5);
+auto rbt4 = redBlackTree!"a > b"(0, 1, 5, 7);
+auto rbt5 = redBlackTree!("a > b", true)(0.1, 1.3, 5.9, 7.2, 5.9);
+--------------------
+ +/
+auto redBlackTree(E)(E[] elems...)
+{
+ return new RedBlackTree!E(elems);
+}
+
+/++ Ditto +/
+auto redBlackTree(bool allowDuplicates, E)(E[] elems...)
+{
+ return new RedBlackTree!(E, "a < b", allowDuplicates)(elems);
+}
+
+/++ Ditto +/
+auto redBlackTree(alias less, E)(E[] elems...)
+{
+ return new RedBlackTree!(E, less)(elems);
+}
+
+/++ Ditto +/
+auto redBlackTree(alias less, bool allowDuplicates, E)(E[] elems...)
+ if(is(typeof(binaryFun!less(E.init, E.init))))
+{
+ //We shouldn't need to instantiate less here, but for some reason,
+ //dmd can't handle it if we don't (even though the template which
+ //takes less but not allowDuplicates works just fine).
+ return new RedBlackTree!(E, binaryFun!less, allowDuplicates)(elems);
+}
+
+//Verify Examples.
+unittest
+{
+ auto rbt1 = redBlackTree(0, 1, 5, 7);
+ auto rbt2 = redBlackTree!string("hello", "world");
+ auto rbt3 = redBlackTree!true(0, 1, 5, 7, 5);
+ auto rbt4 = redBlackTree!"a > b"(0, 1, 5, 7);
+ auto rbt5 = redBlackTree!("a > b", true)(0.1, 1.3, 5.9, 7.2, 5.9);
+}
+//Combinations not in examples.
+unittest
+{
+ auto rbt1 = redBlackTree!(true, string)("hello", "hello");
+ auto rbt2 = redBlackTree!((a, b){return a < b;}, double)(5.1, 2.3);
+ auto rbt3 = redBlackTree!("a > b", true, string)("hello", "world");
}
unittest
{
- RedBlackTree!uint rt1;
- RedBlackTree!int rt2;
- RedBlackTree!ushort rt3;
- RedBlackTree!short rt4;
- RedBlackTree!ubyte rt5;
- RedBlackTree!byte rt6;
+ auto rt1 = redBlackTree(5, 4, 3, 2, 1);
+ assert(rt1.length == 5);
+ assert(array(rt1[]) == [1, 2, 3, 4, 5]);
+
+ auto rt2 = redBlackTree!"a > b"(1.1, 2.1);
+ assert(rt2.length == 2);
+ assert(array(rt2[]) == [2.1, 1.1]);
+
+ auto rt3 = redBlackTree!true(5, 5, 4);
+ assert(rt3.length == 3);
+ assert(array(rt3[]) == [4, 5, 5]);
+
+ auto rt4 = redBlackTree!string("hello", "hello");
+ assert(rt4.length == 1);
+ assert(array(rt4[]) == ["hello"]);
}
version(unittest) struct UnittestMe {
Something went wrong with that request. Please try again.