webmozart / key-value-store

A key-value store API with implementations for different backends.
MIT License
123 stars 14 forks source link

Implement Sortable where possible #8

Closed webmozart closed 9 years ago

webmozart commented 9 years ago

An interface Sortable should be added to the API with the following method:

public function sort($flags = SORT_REGULAR);

This method sorts the store by its keys.

The supported sort flags are the standard PHP sort flags constants (http://de.php.net/sort).

Research needs to be done about which stores support sorting.

tgalopin commented 9 years ago

Here are what I have in mind:

I'm not sure about the others (PhpRedisStore, PredisStore, RiakStore), do you have an idea?

webmozart commented 9 years ago

Sounds good! I don't know about the others, did you research whether they support anything like that?

tgalopin commented 9 years ago

I never worked with Riak but it seems not possible (http://stackoverflow.com/questions/7601997/how-to-sort-order-data-in-riak).

In Redis (PhpRedis and Preedis) it seems possible (http://stackoverflow.com/questions/5780365/redis-how-can-i-sort-my-hash-by-keys) but need a bit of changes in the store, like probably creating a single key containing all the store data.

I'll think about it a bit more.

tgalopin commented 9 years ago

Actually it seems possible to list all the keys in Riak: http://www.paperplanes.de/2011/12/13/list-all-of-the-riak-keys.html. We could store these keys in an array in the store and sort them on sort() call. What do you think?

webmozart commented 9 years ago

Probably that's better, yes. What about changing the interface to:

interface SortedStore extends KeyValueStore
{
}

This interface could be implemented directly by the stores that support sorting easily. For the others, we could implement a SortingAdapter that takes another store and sorts the keys returned from getMultiple() and keys(). What do you think?

webmozart commented 9 years ago

However, as a user of the SortedStore, you lose the power to control the direction of the sorting, which is maybe not optimal. Hm.

tgalopin commented 9 years ago

I think what could be great is SortableStore: just an interface for stores supporting sort. We could also introduce your idea of SortingAdapter as a way to sort not sortable stores. We could have something like:

class ArrayStore implements SortableStore, SortingAdapter
{}

class RiakStore implements SortableStore
{
    private $sortableAdapter;

    public function sort() {
        // We inject all the data in the adapter, sort it, and return
    }
}

What do you think?

webmozart commented 9 years ago

Sounds good. However, I think SortingAdapter should be something like this:

class SortableAdapter implements SortableStore
{
    private $innerStore;

    private $flags = SORT_REGULAR;

    public function sort($flags)
    {
        $this->flags = $flags;
    }

    public function keys()
    {
        $keys = $this->innerStore->keys();
        sort($keys, $this->flags);

        return $keys;
    }
}
tgalopin commented 9 years ago

Hum. I'm not a fan of giving the possibility to use it externaly easily.

Perhaps externalizing the system of SortableAdapter of the stores would be better, don't you think?

webmozart commented 9 years ago

I don't understand?

tgalopin commented 9 years ago

I mean implementing SortableStore make me think it's a store usable from a developer point of view. I think it's more an internal system, and shouldn't implement an API interface. But it's not that much a problem :) .

webmozart commented 9 years ago

The problem is that sorting stores that don't support sorting natively might be quite expensive. So sorting their keys in memory might be a better alternative. Let's talk about this on IRC.

tgalopin commented 9 years ago

For summary:

SortableDecorator::$flags should be null by default (no sorting) and be reset to null on set() so that it behaves the same as any other sorted store, which is only sorted on a call to sort().

Is it fine for you?

webmozart commented 9 years ago

Almost. I would not check whether the inner store implements SortableStore in SortableDecorator, that's too complicated. Simply treat the inner store as non-sortable store.

tgalopin commented 9 years ago

SortableDecorator::$flags should be null by default (no sorting) and be reset to null on set() so that it behaves the same as any other sorted store, which is only sorted on a call to sort().

tgalopin commented 9 years ago

I suppose we can close that now :) .

webmozart commented 9 years ago

Yes, thanks :)