Homework 0 forum

Order of discovered nodes

 
Picture of Peter Krcmar
Order of discovered nodes
by Peter Krcmar - Monday, 21 September 2020, 22:32
 

Hi, I was wondering if the nodes returned by GetNodes should be ordered in some way. I couldn't find anything in the readme or handout but the tests seem to indicate that they should appear from "oldest to most recently discovered". I wanted to know if it was important as right now my implementation doesn't return them in a deterministic order, so I had to change the tests to check for membership instead of checking if they're at a given index, for example I replaced:
require.Equal(t, "127.0.0.1:2001", g2.GetNodes()[1])
by
require.Contains(t, g2.GetNodes(), "127.0.0.1:2001")

Picture of Cristina Basescu
Re: Order of discovered nodes
by Cristina Basescu - Tuesday, 22 September 2020, 13:13
 

Hi,

Strictly speaking, you're right that the handouts don't specify the order, and thus the order should be irrelevant for the test. If you could verify that your test passes after you made the change you suggested, then don't worry, you'll get the points for the test. There's no need to change your implementation to preserve the order.

In the interest of not changing the tests, we won't push a change for this assignment. However, we will keep this in mind for the next assignments.

Thanks a lot for reporting the issue.

Cristina


Picture of Peter Krcmar
Re: Order of discovered nodes
by Peter Krcmar - Tuesday, 22 September 2020, 13:42
 

Great, thanks a lot !

Picture of Bastien Wermeille
Re: Order of discovered nodes
by Bastien Wermeille - Friday, 25 September 2020, 21:14
 
Hello,

I have checked as you asked if sorting the nodes in ascending order would fix this problem but it doesn't. My solution returns the nodes in an unpredictable order for performance reasons so it would be great if the tests could be upgraded to take into account this fact.
It thinks that the tests assumes that the order correspond to the order of addition of those nodes.

Best regards,
Bastien
Picture of Cristina Basescu
Re: Order of discovered nodes
by Cristina Basescu - Friday, 25 September 2020, 23:49
 

Hi,

The order of nodes is irrelevant and the change your colleague suggested fixes the issue, i.e., replacing "require.Equal(t, "127.0.0.1:2001", g2.GetNodes()[1])" with "require.Contains(t, g2.GetNodes(), "127.0.0.1:2001")". As I mentioned above, if you pass the test with this change, it's totally fine.

Perhaps I misunderstood your question?

Cristina

Picture of Bastien Wermeille
Re: Order of discovered nodes
by Bastien Wermeille - Saturday, 26 September 2020, 01:29
 

Sorry, my question wasn't clear.

What I wanted to ask is wether you will you do a patch for the tests or not ?

Bastien