mbeddr / mbeddr.core

The mbeddr core. An extensible C
Eclipse Public License 2.0
224 stars 77 forks source link

Provide utility class for breadth-first traversal and test suite #2436

Closed kbirken closed 1 month ago

kbirken commented 1 month ago

This PR contains the actual implementation Traversal<T> with a lot of javadoc documentation and many test cases.

See the tests for examples how to use it. There are two different APIs which can be used:

This solves #2435.

Here is an example of using the compact API from IETS3.Core:

image
kbirken commented 1 month ago

LGTM. The tests also look correct to me. Please just add an entry to the README.

Thanks - which README do you think of? The one in the root folder doesn't talk about single classes or mpsutils content...

alexanderpann commented 1 month ago

I meant CHANGELOG.md.

dbinkele commented 1 month ago

@kbirken I have tried to improve the test assertions. I am really struggling with the testing of 'nVisited' as it tests the current traversal order of the nodes in the graph. To me this are to strict assumptions on the inner workings of the algorithm. This order especially depends on the order of the start vertices. So, starting with {2,3} can create different 'nVisited' than {3,2}. I added ToDos in the Code highlightning 2 cases.

kbirken commented 1 month ago

@kbirken I have tried to improve the test assertions. I am really struggling with the testing of 'nVisited' as it tests the current traversal order of the nodes in the graph. To me this are to strict assumptions on the inner workings of the algorithm. This order especially depends on the order of the start vertices. So, starting with {2,3} can create different 'nVisited' than {3,2}. I added ToDos in the Code highlightning 2 cases.

According to your todos: I think the order is guaranteed, for a given starting order if there is more than one start node. It is defined by the breadth-first order. With a different starting order there can be a different traversal order, but this will again be deterministic. So from my point of view, the todos can be removed.

BTW: I replaced the type of the visited-collection to linked_hashset now, so that the getVisited() method returns the order of traversal and not an arbitrary one. But this should be unrelated to the problem above. Tests are still working.