Skip to content

Conversation

gordon-klotho
Copy link

Adds the following error structs:

  • VertexAlreadyExistsError[K, T]
  • VertexNotFoundError[K]
  • EdgeAlreadyExistsError[K]
  • EdgeNotFoundError[K]
  • VertexHasEdges[K]
  • EdgeCausesCycleError[K]

This may be breaking if users check err == instead of errors.Is for specific errors (such as err == ErrVertexNotFound and not errors.Is(err, ErrVertexNotFound). Depending on versioning strategy and definition of a breaking change, this may warrant a new major version.

This change also fixes a few issues I found in memoryStore:

  • AddEdge did not conform to the Store interface: "If either vertex doesn't exit, ErrVertexNotFound should be returned for the respective vertex"
  • UpdateEdge had a data race in which the edge could have changed between the s.Edge call and the setting of the edges due to not holding onto the lock.
  • CreatesCycle was not read locking before accessing s.inEdges

To let the store handle error cases when it is part of its interface definition, undirected and directed:

  • AddEdge don't check the vertices for existence
  • RemoveEdge don't check edge existence

And a few minor changes I made while I was there:

  • Use ListEdges instead of AdjacencyMap for Size, saves some allocations and another pass through the edges to create the map
  • undirected copy directed's createsCycle method to delegate to the store's version for a more performant implementation

@dominikbraun dominikbraun self-requested a review October 13, 2023 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant