Skip to content

Peer review #7

@0tso

Description

@0tso

I'll divide the peer review into three sections.

  1. Pressing issues
  2. User experience suggestions
  3. Program optimization & structure

Pressing issues

The program does not start if you follow the installation instructions written in user_guide.md. This is because the program is configured to use .NET 6.0 instead of the "latest .NET" (=8.0) that the user guide instructs you to install. I could get the program to work by modifying the <TargetFramework> parameter in PathFinder.csproj.

As you perhaps already know, the algorithms provided do not work consistently. For example on the provided test map London_1024x1024.map JPS returns a different path length than A* and Dijkstra with the following start & end coordinates: (500, 321) -> (951, 811).

User experience suggestions

It would be nice to see the starting and ending points after the pathfinding has been done, because for larger maps such as the aforementioned London map, the printed output completely fills the console.

Program optimization & structure

The algorithm modules (Astar.cs, Dijkstra.cs, JPS.cs) have multiple duplicated functions such as GetVisitedNodes, GetStopwatchTime and Heuristic that could be separated into their own base class that the algorithm modules inherit from.

The nodes themselves could be lazily initialized to save a lot of time in larger maps. You can compare the times achieved by A* and Dijkstra on larger maps like London_1024x1024.map and scenarios where the start and end points are near each other. In such situations (for example (600, 600) -> (605, 605)) A* is 1000x slower than pure Dijkstra because you initialize the whole map in the beginning of the algorithm.

In a grid map with 8-directional movement such as the one you are using, octile distance as a heuristic is significantly faster than euclidian distance according to my own benchmarks (because it more closely matches the true path lengths themselves). Therefore I recommend changing your heuristic to that.

The usage of string resources is not very consistent: some strings are saved in AllStrings.resx, but most are not.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions