incremental-topo icon indicating copy to clipboard operation
incremental-topo copied to clipboard

Reconsider API design

Open declanvk opened this issue 3 years ago • 0 comments

Some rambling ideas:

In the changes from v0.1.2 to v0.2.0, the signature of the IncrementalTopo type changed from IncrementalTopo<T> to IncrementalTopo, losing the ability to store user-defined types. This was largely because I didn't want to have the bi-map component anymore, and thought that it would be better for performance just to hand out tokens which represented nodes in the graph.

The performance was a bit better, but made using the library more difficult. The expression evaluator example was the main usage test for the library. The changes to the example to work with the v0.2.0 made me realize that maybe having some user-defined type stored in the IncrementalTopo object might be useful.

Previously, the type T in the IncrementalTopo<T> signature represented the node type. When creating a new node, the user would specify a value of type T which uniquely represented the node. Then, the other APIs had the form:

pub fn method_using_single_node<Q>(&self, node: &Q) -> bool
    where
        T: Borrow<Q>,
        Q: Hash + Eq + ?Sized;

This gave users the flexibility to have a IncrementalTopo<String>, then using &str form to query it.

Instead of reverting to this, what if I made a new API that allowed store abitrary T values, but returned a opaque node token (like the incremental_topo::Index value today). Other APIs would still take the token to query and mutate dependency information. The query and iterator APIs could return references to T or &T or &mut T depending.

I think this would simplify the expr_evaluator example specifically because it could remove the bindings and values maps from https://github.com/declanvk/incremental-topo/blob/90dd2a26f19c3b741eb1e4220b5de82b458ca70f/examples/expr_evaluator.rs#L115-L120.

declanvk avatar Aug 20 '22 06:08 declanvk