Algorithms to find various kinds of paths in graphs

I think many here are familiar with the graph data structure. Not very long ago, I implemented as an exercise a supposedly simple Graph application that does some traversals. To note, the most complex operations in this class are those that exhaustively find Paths: findDirectPaths, findPathsWithMaximumDistance, and findPathsWithMaximumStops.

Other collaborators such as Node, and Edge have very simple structure that the reader should be able to guess from their usage in this class. However, for those who are curious about the overall structure of this application, they can check out the public repository on GitHub.

My work has recently been assessed, and I was given remarks similar to the following:

• There is insufficient test coverage with no evidence of Test Driven Development (TDD).

• There are quite a number of unused methods.

• Methods are unnecessarily long, and almost all logic is in the Graph class.

Further, according to the people who reviewed it, while my solution works and has basic test coverage, the methods are large and difficult to follow. They encourage me to follow clean code principles, and demonstrate object oriented design.

Now, I'd like for other people to review my code and see what else should be improved here and how. I'd also like to know if the assessments above are well grounded because I don't necessarily agree with them.

To the first point, I'm not really sure what kind of evidence anyone can present that there was actually TDD involved in building the solution. Isn't that there are tests for almost every method proof enough that there actually was TDD involved here (regardless of whether it was done properly, or not)? My code base was sent in as a compressed directory so version control data was removed, which should have illustrated the step by step building of each of the methods, in chronological order. I make it a habit to commit changes after test-fail-code-pass sequence.

As for the unused methods, I had to r

So the first thing I noticed:

if (destinationEdges == null) graphMap.put(destination, new ArrayList());
return edge;

I thought why return edge if edge == null

Then I noticed you use multiple styles. That got me really confused.

I would have assumed it to be like this instead based on the style you use:

if (destinationEdges == null) 
    graphMap.put(destination, new ArrayList());

return edge;

hasEdge is clearly something you used for unit testing, and should not exist in the main class.

Also why are the findEdge functions not both private or public?

And the following functions are way to big in my opinion.

Sometimes this is unavoidable, but I don't feel that this is the case:

• findPathsWithMaximumStops

• findPathsWithMaximumDistance

• findDirectPaths

I feel that in the functions above the big Loops could partly be split of into a single function, making it easier to read and reuse.

StackExchange Code Review Q#129084, answer score: 3