Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Min/Max values incorrect #28

Closed
nordic81 opened this issue Feb 19, 2021 · 6 comments
Closed

Min/Max values incorrect #28

nordic81 opened this issue Feb 19, 2021 · 6 comments

Comments

@nordic81
Copy link

If I add an interval to the tree that spans around the center value of the tree providing a new over all minimum and or maximum key value, the provided min/max values of the tree are wrong.
The root node will only look in the outer subtrees, if these exist and ignore the center items.

@apacha
Copy link
Collaborator

apacha commented Feb 19, 2021

Thanks for reporting this issue. Can you give us a minimum executable example, so we can reproduce and fix the issue?

@nordic81
Copy link
Author

i was just trying to push the tests and the fix in a separate branch, but obviously I'm not contributor enough :D

`
[Test]
public void GettingMin_Mixed()
{
var tree = new IntervalTree<int, int>
{
{ 2, 3, -1 },
{ 8, 9, -1 },
{ 1, 10, -1 },
};

var min = tree.Min;

Assert.That(min, Is.EqualTo(1));

}

[Test]
public void GettingMax_Mixed()
{
var tree = new IntervalTree<int, int>
{
{ 1, 10, -1 },
{ 2, 3, -1 },
{ 4, 5, -1 },
{ 8, 9, -1 },
};

var max = tree.Max;

Assert.That(max, Is.EqualTo(10));

}
`

Suggested fix:
IntervalTreeNode.cs
`
public TKey Max { get; private set; } = default(TKey);

public TKey Min { get; private set; } = default(TKey);
`

IntervalTreeNode cosntructor:
// the median is used as center value if (endPoints.Count > 0) { Min = endPoints[0]; center = endPoints[endPoints.Count / 2]; Max = endPoints[endPoints.Count - 1]; }

@apacha
Copy link
Collaborator

apacha commented Feb 19, 2021

Thanks for the example. I see your point.

Yes, you do not have push-permissions to this repository. The correct flow, how you can contribute directly by opening a Pull-Request can be found here: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request.

If you create a PR, I can review and merge it.

@nordic81
Copy link
Author

overread the part with the fork. thanks. :)

@nordic81
Copy link
Author

nordic81 commented Mar 9, 2021

any updates on this matter? 👼

@apacha
Copy link
Collaborator

apacha commented Apr 29, 2021

Fixed in 3.0.1

@apacha apacha closed this as completed Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants