search

ProAlgos / ProAlgos-Cpp

C++ implementations of well-known (and some rare) algorithms, while following good software development practices
MIT License
508 stars 363 forks source link

Added Heap #324

Closed MarceloMoraesJr closed 4 years ago

MarceloMoraesJr commented 4 years ago

Added a heap tree data structure based on comparison function passed by template parameter.

Closes #323 [contrib-guidelines]: https://github.com/faheel/Algos/blob/master/CONTRIBUTING.md [unit-tests]: https://github.com/faheel/Algos/blob/master/C%2B%2B/UNIT_TESTS.md [contents]: https://github.com/faheel/Algos/tree/master/C%2B%2B#contents
alxmjo commented 4 years ago

I should be able to take a look at this today or tomorrow, but in the meantime, could you convert the tabs to spaces (one tab = 4 spaces)? Thanks!

alxmjo commented 4 years ago

I'm still seeing plenty of tabs in your files, both in heap.hpp and heap.cpp. You should be able to use your IDE to convert these all at once. Also, in heap.hpp, lines 94 through 96, I also noticed a place where the double indentation was 7 spaces instead of 8. Please make sure these are fixed.

As for the tests in heap.cpp, have a look at how we implemented the tests in binary_search_tree.cpp. You'll want to make sure you use this same syntax (which draws on Catch.hpp), and that you're testing the equivalent functions in heap.hpp (insert, remove, etc.).

It would also be a good idea to use the same naming conventions as binary_search_tree.hpp, like insert() instead of Push_Heap(), etc. And note the capitalization, too. Let's keep them all lower case.

Let me know if you have any questions!

alxmjo commented 4 years ago

I see several people contributing to this PR. Out of curiosity, do you all know each other? Is this part of the class that Paolo was talking about?

MarceloMoraesJr commented 4 years ago

Hi, i apologize for the delay in answering, it was a busy weekend. First, i would like to say thanks for your feedback on our PRs. Yes, we're part of Paulo's class, i'm sorry, i should have said that before. About the PR, we will try to fix it as soon as possible. Thank you for understanding!

alxmjo commented 4 years ago

No problem, and no rush. 🙂

stale[bot] commented 4 years ago

This issue has been automatically marked as inactive because it has not had recent activity. It will be closed in 15 days if no further activity occurs. Thank you for your contributions.