Currently the user has to implement the Node interface, which requires him to implement several methods, like getID, setParent, getParent, getF, and so on. Luckily, the user can extend from AbstractNode, which implements everything but getID.
However, this is just highlighting the fact that the only user-related method in the Node interface is getID, the rest of the methods are algorithm-related, and they shouldn't be exposed to the user.
The user shouldn't be accessing algorithm-related methods, it's not his responsibility to change the G or H value of a node, for instance; the algorithm should take care of that.
There are other problems related to the current approach. For instance, when the user extends the AStar class, he has to implement three methods (generateAdjacentNodes, calculateRealCost and calculateEstimatedCost), all of them have Nodes as parameters. If the user changed any of the Node parameters (for instance, the parent, or the G value) while he's implementing one of the previous three methods, it could lead to a fail in the algorithm.
Possible solution:
interface NodeIdentifier
{
public function getUniqueID();
}
class Node
{
// This is pretty much the current AbstractNode, plus the following:
private $userData;
public function __construct(NodeIdentifier $userData)
{
$this->userData = $userData;
}
public function getID()
{
return $this->userData->getUniqueID();
}
public function getUserData()
{
return $this->userData;
}
}
So now the user won't create a class that extends from AbstractNode (as it won't exist anymore), his class should just implement NodeIdentifier. For instance, instead of MyNode extends AbstractNode, it would be MyNode implements NodeIdentifier. Then, when the user extends from AStar, the three methods' parameters won't be of type Node, they will be of type MyNode.
As this is a major change and it would break backwards compatibility, the major version of the library should be increased as well.
Currently the user has to implement the
Node
interface, which requires him to implement several methods, likegetID
,setParent
,getParent
,getF
, and so on. Luckily, the user can extend fromAbstractNode
, which implements everything butgetID
.However, this is just highlighting the fact that the only user-related method in the
Node
interface isgetID
, the rest of the methods are algorithm-related, and they shouldn't be exposed to the user.The user shouldn't be accessing algorithm-related methods, it's not his responsibility to change the G or H value of a node, for instance; the algorithm should take care of that.
There are other problems related to the current approach. For instance, when the user extends the
AStar
class, he has to implement three methods (generateAdjacentNodes
,calculateRealCost
andcalculateEstimatedCost
), all of them have Nodes as parameters. If the user changed any of the Node parameters (for instance, the parent, or the G value) while he's implementing one of the previous three methods, it could lead to a fail in the algorithm.Possible solution:
So now the user won't create a class that extends from
AbstractNode
(as it won't exist anymore), his class should just implementNodeIdentifier
. For instance, instead ofMyNode extends AbstractNode
, it would beMyNode implements NodeIdentifier
. Then, when the user extends fromAStar
, the three methods' parameters won't be of typeNode
, they will be of typeMyNode
.As this is a major change and it would break backwards compatibility, the major version of the library should be increased as well.