patternjavaMinor
Live Graphical Demonstation of a Breadth-First Search Algorithm
Viewed 0 times
searchbreadthalgorithmlivefirstgraphicaldemonstation
Problem
As a first step to making a PacMan clone, I wrote a Breadth-First Search demonstration that updates itself live as you draw walls and drag around the start and end points. It was inspired by the site that I learned the algorithm through, which has similar demonstrations.
A sample of it:
To use it, use the mouse to drag the start (black) and end (pink) points, or draw on empty space to draw a wall.
My Main Concerns:
-
This is my first crack at a pathfinding algorithm, so I'm sure there is room for improvement. Any tips here would be appreciated!
-
Allowing canvas elements to be dragged was more painful than I though it'd be. I opted to use a global
-
Anything else that you think could be improved upon
A Couple Things to Note:
-
This demo is fairly rough around the edges. Attempting to draw out-of-bounds fails silently in the application, but results in a
-
The demo code uses a
FrontierVisual.java:
```
package pacmanTest;
import java.util.ArrayList;
import java.util.List;
import javafx.application.Application;
import javafx.event.EventHandler;
import javafx.scene.Scene;
import javafx.scene.canvas.Canvas;
import javafx.scene.canvas.GraphicsContext;
import javafx.scene.input.MouseEvent;
import javafx.scene.layout.BorderPane;
import javafx.scene.paint.Color;
import javafx.scene.text.Font;
import javafx.stage.Stage;
import utils.Duple;
import utils.MainLoop;
/**
* To record what's currently being dragged
*/
enum DragPoint {
START, GOAL, NONE
}
public class FrontierVisual
extends Ap
A sample of it:
To use it, use the mouse to drag the start (black) and end (pink) points, or draw on empty space to draw a wall.
My Main Concerns:
-
This is my first crack at a pathfinding algorithm, so I'm sure there is room for improvement. Any tips here would be appreciated!
-
Allowing canvas elements to be dragged was more painful than I though it'd be. I opted to use a global
enum member to track what's currently being held, but it feels like a hack. Is there a more standard way of moving non-node elements?-
Anything else that you think could be improved upon
A Couple Things to Note:
-
This demo is fairly rough around the edges. Attempting to draw out-of-bounds fails silently in the application, but results in a
NoSuchElement exception being thrown behind the scenes.-
The demo code uses a
Wall class, but it currently doesn't do anything. Since the application only cares about what cells are occupied and assumes that any occupied cell is a wall, the private member wall can be replaced by any type (A number type for example), and it will still work fine.FrontierVisual.java:
```
package pacmanTest;
import java.util.ArrayList;
import java.util.List;
import javafx.application.Application;
import javafx.event.EventHandler;
import javafx.scene.Scene;
import javafx.scene.canvas.Canvas;
import javafx.scene.canvas.GraphicsContext;
import javafx.scene.input.MouseEvent;
import javafx.scene.layout.BorderPane;
import javafx.scene.paint.Color;
import javafx.scene.text.Font;
import javafx.stage.Stage;
import utils.Duple;
import utils.MainLoop;
/**
* To record what's currently being dragged
*/
enum DragPoint {
START, GOAL, NONE
}
public class FrontierVisual
extends Ap
Solution
Separate Model from Rendering
The
They should be separated. All JavaFx-related stuff would remain in
Don't Abuse of Generics
Please look closer at these declarations:
Do you find this pollution with generics, spread everywhere in the code, useful, readable and aesthetically pleasant?
I guess that what you are trying to do here is to anticipate a probable future replacement of
Update. To reduce this overflow of generics, that may be useful to transform
Similar changes may be done with the other generics used... Genericity is good, of course, but it should be pondered if it's really necessary here.
Consider Using Existing API
The
p.s. Please change package names to something that respects Java naming conventions.
The
FrontierVisual class contains at least two semantically distinct groups of fields:- Visual component related properties, like
stage,scene,areaWidth,areaHeightanddrawScale.
- Virtual space definition, like
area,pathFinder,startPosetc.
They should be separated. All JavaFx-related stuff would remain in
FrontierVisual class and the model would be better described within a dedicated entity. For small applications like this one, there is probably no need to implement fully the Model-View-Controller pattern, but at least model-view separation would be beneficial, bringing more flexibility and extensibility.Don't Abuse of Generics
Please look closer at these declarations:
Map, Duple> cameFrom = ...
List> path = ...Do you find this pollution with generics, spread everywhere in the code, useful, readable and aesthetically pleasant?
I guess that what you are trying to do here is to anticipate a probable future replacement of
Integers with Doubles and so on.Update. To reduce this overflow of generics, that may be useful to transform
Duple into an abstract class and add a short one that inherits from it with the right numeric type. But I doubt that this is necessary at all. Your virtual space is better defined using integers. Why not just have a Point class with int x, y? That will avoid boxing/unboxing with Integer in many cases.Similar changes may be done with the other generics used... Genericity is good, of course, but it should be pondered if it's really necessary here.
Consider Using Existing API
The
Duple class seems to have a number of features similar to java.awt.Point or javafx.geometry.Point2D. They may be considered for reuse here.p.s. Please change package names to something that respects Java naming conventions.
Code Snippets
Map<Duple<Integer>, Duple<Integer>> cameFrom = ...
List<Duple<Integer>> path = ...Context
StackExchange Code Review Q#108048, answer score: 4
Revisions (0)
No revisions yet.