Skip to content
This repository has been archived by the owner on Dec 7, 2019. It is now read-only.

Add ClearDisk interface #102

Closed
digitalbuddha opened this issue Feb 2, 2017 · 10 comments
Closed

Add ClearDisk interface #102

digitalbuddha opened this issue Feb 2, 2017 · 10 comments
Assignees

Comments

@digitalbuddha
Copy link
Contributor

in conjunction with #68 we should have a way to clear memory and disk cache. Current idea is add:

Store:
void clear(BarCode barCode);
Builder:
builder.diskClearer(barcode->//deletes entry from user's persister);

@pavlospt
Copy link
Contributor

pavlospt commented Feb 2, 2017

@digitalbuddha it should also provide an interface for whole disk cache clear I suppose. But that I guess would be exposed internally from the FileSystem ?

@PaulWoitaschek
Copy link
Contributor

Why would one want to clear one without the other? (cache & persistence)

Why shouldn't it be mandatory for persisters to be able to clear their data?

@digitalbuddha
Copy link
Contributor Author

For now backwards compatibility. I'll Deprecated and consider removing in v2.

@PaulWoitaschek
Copy link
Contributor

What about defining a common interface like Clearable that both the cache and the persister can implement? Then upon calling .clear throwing a NotSupportedException if the persister is not an instance of Clearable?

@digitalbuddha
Copy link
Contributor Author

Not following, mind explaining what you mean? Also the memory cache is currently a guava cache which we don't have control over interface. I guess it can be wrapped but yeah mind explaining what you mean?

@PaulWoitaschek
Copy link
Contributor

If I understand you correctly, you want to give the option to add a DiskClearer to the store that you supply through the builder.
This class responsibility is to clear data from the Persister.

So my suggestion is to instead of having a separate DiskClearer to have an interface that Persisters can implement to indicate that they know how to clear data.
Then if someone would call clear without his Persister implmenting Clearable an NotSupportedException("The supplied persister must implement Clearable")

Or do I get you completely wrong?

@digitalbuddha
Copy link
Contributor Author

Sure that makes sense. I was trying to handle the lambda case in the way that DiskRead & DiskWrite exist for when a Store is instantiated. So maybe instead it should just be an additional override to .persister(reader, writer, clearer) while also allowing the interface to be implemented by someone implementing the current Persister interface. Thanks for feedback!

@digitalbuddha
Copy link
Contributor Author

@PaulWoitaschek I'm starting to side with you. If there is a memory and disk cache why would someone only want to clear the memory one? I'll instead rename clearMemory to clear and then clear disk when the disk implements clearable thank you for feedback

@pavlospt
Copy link
Contributor

pavlospt commented Feb 8, 2017

@digitalbuddha Is there a chance to have memory and disk cache side-by-side?

@digitalbuddha
Copy link
Contributor Author

You can have both but I don't see a use case where you want to clear your memory cache but not your disk. If that is your use case you can always not implement clearable in your persister

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants