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

MaxHeap bad algorithm. #84

Open
crucialize opened this issue Dec 28, 2018 · 2 comments
Open

MaxHeap bad algorithm. #84

crucialize opened this issue Dec 28, 2018 · 2 comments
Labels
bug Report an issue with the project good first issue

Comments

@crucialize
Copy link

steps to reproduce

Write a loop, from 1 to 80000, each time add a random int to the max heap.

In theory it takes very little time(NlogN, N=80000, <1sec ), but the program does take a long time.

I'v also tested the BinaryHeap in https://github.com/SolutionsDesign/Algorithmia, it performs well, so it is probably due to the bad algorithm.

@aalhour aalhour reopened this Nov 1, 2019
@aalhour aalhour added the bug Report an issue with the project label Nov 1, 2019
@aalhour aalhour added this to To do in Kanban Board via automation Nov 1, 2019
@aalhour
Copy link
Owner

aalhour commented Nov 1, 2019

@crucialize - would you like to submit a PR for this?

@davidsonbrsilva
Copy link

davidsonbrsilva commented Nov 6, 2019

Good afternoon! So, I've tested the BinaryMaxHeap class according the steps above and here the execution was perfect. For testing, I've created a execution project with a Main Class program with the follow code:

BinaryMaxHeap<long> maxHeap = new MaxHeap<long>(Comparer<long>.Default);
Random rand = new Random();

/* Adding the elements to heap. */
for (int i = 0; i < 80000; ++i)
{
    maxHeap.Add(rand.Next());
}

/* Removing the elements from heap. */
while (!maxHeap.IsEmpty)
{
    var element = maxHeap.ExtractMax();

    /* Showing part of heap. */
    if (maxHeap.Count % 2000 == 0)
    {
        Console.WriteLine(element);
    }
}

Maybe the found problem is related to execution time to generate random numbers, it's my guess. If a random object is created each time a random number is generated, putting the instantiation line inside loop affects performance.

Hope this helps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report an issue with the project good first issue
Projects
Kanban Board
  
To do
Development

No branches or pull requests

3 participants