You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
usingSystem;usingSystem.Collections.Generic;classX{publicstaticvoidMain(){varset=newSortedSet<int>{1,3,5,7,9};varview=set.GetViewBetween(4,8);Console.WriteLine(view.Max);// Prints 9 instead of 7}}
Sorry. So the previous code for Min and Max was using InOrderTreeWalk, which got overridden by TreeSubset.
There look like there are three possible fixes,
write a custom Min and Max for TreeSubSet,
use the old version from Min and Max, from before my change, in TreeSubSet, or
revert completely, and take the extra allocation, for Min and Max.
Can/should subclasses depend on Min and Max calling InOrderTreeWalk? It is internal, so as long as we find all subclasses in CoreFX then it should be okay to do 1 or 2. I am happy to do 1 or 2.
2 is the simplest, 1 would be the most optimal. What is the time line for needing the fix?
Let's do whichever strikes a decent balance between not being too much worse on perf and minimizing the chance of further regression. Sounds like that's 2?
Activity
stephentoub commentedon Mar 6, 2017
cc: @mjp41, @ianhays
mjp41 commentedon Mar 6, 2017
Sorry. So the previous code for
MinandMaxwas usingInOrderTreeWalk, which got overridden byTreeSubset.There look like there are three possible fixes,
Can/should subclasses depend on
MinandMaxcallingInOrderTreeWalk? It isinternal, so as long as we find all subclasses in CoreFX then it should be okay to do 1 or 2. I am happy to do 1 or 2.2 is the simplest, 1 would be the most optimal. What is the time line for needing the fix?
stephentoub commentedon Mar 6, 2017
Relatively soon :)
Let's do whichever strikes a decent balance between not being too much worse on perf and minimizing the chance of further regression. Sounds like that's 2?