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

Expose MemoryStore #164

Open
davidovich opened this issue Dec 20, 2023 · 1 comment · May be fixed by #165
Open

Expose MemoryStore #164

davidovich opened this issue Dec 20, 2023 · 1 comment · May be fixed by #165
Labels
enhancement New feature or request

Comments

@davidovich
Copy link

At $WORK, we implement our own Store interface for reading existing db entities into a graph. This store only implements Read related interface functions of the Store interface and works nicely.

We now would like to implement a partial Store with only Write related Store interface and reuse existing implementation from the existing default memoryStore. For example, I would like to be able to write:

Note that generics were omitted in the following to lighten the code.

// myGraphWriter implements the graph.Store interface because it embeds a MemoryStore
type myGraphWriter struct {
	graph.MemoryStore // this isn't possible because the memoryStore is private
}

// We selectively (or partially) implement writer related functions
func (g *myGraphWriter) AddVertex(hash K, value T, properties VertexProperties) error {
	v := g.MemoryStore.AddVertex() // memory dispatch
	// do proprietary db stuff here
}

Because the current memoryStore is private, we would need to reimplement our memory store just to override write-related functions. Embedding nicely fixes that - but memoryStore should be exposed.

I propose we make the memoryStore public: memoryStore -> MemoryStore. Fields of the MemoryStore may stay private.

@davidovich davidovich linked a pull request Dec 20, 2023 that will close this issue
@dominikbraun dominikbraun added the enhancement New feature or request label Feb 28, 2024
@dominikbraun
Copy link
Owner

Sorry for the late response, I had a busy time at work and I'm back now. This sounds reasonable, going to review the PR!

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

Successfully merging a pull request may close this issue.

2 participants