proposal: graph: remove Has methods
Created by: kortschak
Background
I have just updated the graph API to address a long standing issue of a subtle and potentially surprising intereaction between node and edge retrieval from graphs. In order to allow users to manage this subtlety I needed to add a method to retrieve a node from the graph — as distinct from obtaining a potentially new node via the NewNode
method. This means there are now two approaches to determining whether a node exists in the graph:
exists := g.Has(id)
and
exists := g.Node(id) != nil
When I added the Node
method, I was aware of this, but my thinking was that g.Has
is potentially cheaper than returning a node. However, whether this is significant, or even true, is not something I looked at. I doubt in many cases whether either of these is true.
The existence of two approaches to obtaining the knowledge of existence of a node allows skew between them and reduces code consistency. I also think that while g.Node(id) != nil
is longer it arguably more clearly states what is being asked to the graph.
Proposal
I would like to suggest that the Has
method be removed from the Graph
and Multigraph
interfaces and all implementations we provide.
Potential impact of proposal
This will be quite an invasive, but largely mechanical change (made less amenable to mechanisation by the existence of a Has
method on our set types).
It is however a breaking change since Has
has been an integral part of the graph API and node existence testing is central to many graph querying algorithms.
I will do the work.
/cc @vladimir-ch @mewmew