Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconsider API design #28

Open
declanvk opened this issue Aug 20, 2022 · 0 comments
Open

Reconsider API design #28

declanvk opened this issue Aug 20, 2022 · 0 comments
Labels
enhancement New feature or request

Comments

@declanvk
Copy link
Owner

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

#[derive(Debug, Clone, Default)]
struct Assignments {
bindings: HashMap<Symbol, Rc<Expr>>,
ordering: IncrementalTopo,
values: HashMap<Symbol, Value>,
}
.

@declanvk declanvk added the enhancement New feature or request label Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant