Crystal: RFC: Make Indexable writeable

[lbguilherme]

Just exposing something that was on my mind: Currently Indexable requires size and unsafe_at to implement several reading operations on the container. Those are the classes that use Indexable now:

Array(T)   BitArray   Deque(T)   Slice(T)   StaticArray(T, N)   Tuple(*T)

From those, Array, BitArray and Deque are always writeable, Slice and StaticArray are some times writeable (unsafe), Tuple is always read-only. So, Indexable assumes a read-only container and each one of them is responsible for the write operations they want.

Some operations are implementable on all writable indexable: sort, fill, shuffle, update, swap. It would be nice to either offer those operations directly in Indexable itself (bad for Tuple), or create another module for WritableIndexable (maybe with better name), so that all containers can benefit from these functions. This way Array would have less specific things.

Btw, with this change there would be almost no reason to keep Array at all, Deque could be renamed to "Array" as it is at most as fast as Array, and is sometimes faster.

[lbguilherme]

Ooops, just noted @asterite's comment on https://github.com/crystal-lang/crystal/issues/5126#issuecomment-337566280

[ghost]

related from iRC

it is true that sort! and rotate! can be implemented on Indexable and that needs to be brought up

[Papierkorb]

Crystal touts itself to be type-safe. This also means that checks are done as early as possible (Best case: Compile-time). If something can't be writable, it shouldn't expose the methods for it. This is one of the many things that simply suck in dynamic languages. That Slice is using a run-time check is already a defect.

Overall, exposing methods that don't yell at you at compile time is simply bad design, and nothing else.

What we could do, and this would actually be a nice thing, would be to have ReadOnlyIndexable and Indexable (for read/write). Indexable would then simply include ReadOnlyIndexable(T). I'd choose it this way around to make it easier to read, as a read/write container is more common than a read-only one.

[lbguilherme]

Just to be clear, I don't think anymore that it would be a good idea to merge Deque and Array. But that doesn't change the core of this issue, about Indexable.

I like having ReadOnlyIndexable and Indexable as @Papierkorb suggested. If there is nothing against it, I'll follow up with a PR.