Refactored Graph Traversal Implementation

[yashTEF]

The existing findShortestPath() implementation for GraphTraversalService is cumbersome
and hard to read, hence it can be refactored and implemented in a simpler and more efficient manner.

[yashTEF]

There are some other places where redundant parameters are being used.

@Size(min = 8, max = 8, message = "Deadline value must be eight characters long.") @QueryParam("deadline") String deadline)

The above deadline parameter is not needed, as the validation for deadline is done later in the ExternalRouteService
class as

transitPaths .stream() .map(this::toItinerary) .forEach( itinerary -> { if (routeSpecification.isSatisfiedBy(itinerary)) { itineraries.add(itinerary); } else { logger.log( Level.FINE, "Received itinerary that did not satisfy the route specification"); } });

Also in getVoyageNumber() we don't require the parameters
getVoyageNumber(String from, String to)

However, the above are not removed as I assume this is present to simulate real world application
and could be useful there.

If needed these can be removed too.

[m-reza-rahman]

Please revert this. It is inefficient to create an empty list when capacity is known.

[m-reza-rahman]

Please revert this. It is inefficient to create an empty list when capacity is known.

[m-reza-rahman]

The existing findShortestPath() implementation for GraphTraversalService is cumbersome and hard to read, hence it can be refactored and implemented in a simpler and more efficient manner.

How sure are you that this hasn't introduced subtle bugs? This is the logic that came from the original project: https://dddsample.sourceforge.net. It was never intended to be very accurate. As long as it still does something plausible, it's fine to make these changes. Also, feel free to evaluate their current implementation of this logic: https://github.com/citerus/dddsample-core/blob/master/src/main/java/com/pathfinder/internal/GraphTraversalServiceImpl.java.

[m-reza-rahman]

However, the above are not removed as I assume this is present to simulate real world application and could be useful there.

Correct. Let's kindly leave these alone. There is far more important work for us to do.

[m-reza-rahman]

Please address and consider the comments above.

[yashTEF]

Also, feel free to evaluate their current implementation of this logic: https://github.com/citerus/dddsample-core/blob/master/src/main/java/com/pathfinder/internal/GraphTraversalServiceImpl.java.

I have followed up and opened a PR there, the project seems to be active so will update here on any progress.

As for the implementation, there is no change in the logic the only difference being that instead of adding first leg and last leg for the candidate itinerary we add them inside the for loop itself.

How sure are you that this hasn't introduced subtle bugs?

I did some tests manually and it seems to work fine, for unit tests there is an ExternalRoutingServiceTest which contains

public void testCalculatePossibleRoutes() {

which seems to test this but is not in use and needs to be completed.

Maybe this could be a part of #6 ?

I'll make the changes requested meanwhile.

[m-reza-rahman]

OK. Just make the changes please and I will merge and test a bit myself. Yes, the unit tests need help.

[yashTEF]

@m-reza-rahman , The maintainers for the original dddsample project have approved above changes too!
Here's the discussion: citerus/dddsample-core#94

It is passing their set of unit tests so hopefully there shouldn't be any worries of unknown defects creeping in.