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

Added some tests #42

Closed
wants to merge 1 commit into from
Closed

Added some tests #42

wants to merge 1 commit into from

Conversation

armanhbi
Copy link

@armanhbi armanhbi commented Jan 12, 2022

Homework Number: W10H02 (A Heap of Nodes [Bonusaufgabe])

A short description of the changes proposed in the pull request:
I added some tests on every method we should implement. I equal-checked the heaps with the use of the toString() that we should implement, which means that the whole tests are dependent on toString(), which is not the best implementation but for finding your own fault it should be enough. Otherwise an implementation of another comparison algorithm is of course possible to (for example the toList() method or a recursive method going through all nodes and comparing them).

I even implemented a TestObject to check if the student used the compareTo()-method because most of us only used Integers as the generic type, but other generic types are also possible!

Some info on the testDelMin()-method: I know that some of the students implemented it in another way. I asked which of the implementations are right on Zulip today, but did not get any answer yet: (https://zulip.in.tum.de/#narrow/stream/879-PGdP-W10H02--.20A.20Heap.20of.20Nodes/topic/delMin.28.29/near/431120). So please be uptodate with the zulipstream and if my tests are wrong because my implementation is, I will change it of course and submit it again.

P.S.: It is my first time writting tests for this github repo and even the first time creating a pull request and working with github in general, so if I did something wrong I am very sorry and just tell me.
P.S.2: As i already mentioned the tests are far from being programmed in a nice way, so I am sorry for that too. But i guess they should be fine to cover the basic implementation. I hope that they do not reveal any solutions, because the toString() method should not be part of giving any solutions out. (But I am not quite sure...)

  • Solution is not getting revealed
  • Files are formatted

@LadnerJonas
Copy link
Owner

I'm sorry to tell you, but i will not waste any more time on this project. But beside the delMin() test your tests look alright, but do not contain real edge cases. The public tests already check some of your tests. So i will not merge your changes.

Sorry.

@LadnerJonas
Copy link
Owner

Now added this project update on the main page
See fc11b7a
@armanhbi

@LadnerJonas
Copy link
Owner

As the Week is over, i will close down this Pull request

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

Successfully merging this pull request may close these issues.

None yet

3 participants