[8.x] Move contracts

[driesvints]

It doesn't really makes sense that Enumerable is also array accesible or has anything to do with JSON manipulations. These interfaces should be defined on the implementing class.

It does makes sense however to keep Countable and IteratorAggregate as we're still talking about collections here. But we shouldn't force people to also implement ArrayAccess or also any JSON contracts.

Doing this on 8.x with the new collections component is a good moment to do this.

@JosephSilber does this seem reasonable to you?

[JosephSilber]

Is this about semantics, or does it have an actual benefit to some user out there?

• If it's about semantics, then why stop here? Do average, min, max, median, dd, dump (and countless others) semantically belong on an interface named Enumerable?

  If you're bothered by the semantics (I'm not), then let's rather rename this interface to something more generic.

• If this is about real-user benefit: whom does it benefit?

  1. Is anyone really going to implement their own collection class from scratch in userland, using the Enumerable contract? And if they can implement all of the enumerable methods, do we think these few extra other methods are gonna be a burden?

  2. This actually leads to a worse DX: if I have a method that can take either a regular Collection or a LazyCollection, I can currently type-hint it with Enumerable. After this change, that means I can no longer call toArray, toJson and all of those other methods. And for what?

[driesvints]

If it's about semantics

That's a different discussion atm. This is the bare minimum that I did with the current contracts. We did the exact same thing for the container a while back because the container shouldn't be worried with being a way for people to use it as an array, just like collections here.

Is anyone really going to implement their own collection class from scratch in userland, using the Enumerable contract?

That's not really an argument I think because if so then the entire purpose of a contract in the first place is defeated. Using the Enumerable contract with the bare minimum features allows to let us use it in the internals and not exploit any user-land syntactic sugar (which array access is here if we have to be honest).

After this change, that means I can no longer call toArray, toJson and all of those other methods.

In which situation do you need this?

Ps. appreciating the feedback!

[JosephSilber]

That's not really an argument I think because if so then the entire purpose of a contract in the first place is defeated.

Why is that? Having the contract means that, as I said before, I can use the contract's type-hint and accept either collection type.

Using the Enumerable contract with the bare minimum features allows to let us use it in the internals and not exploit any user-land syntactic sugar

How does removing stuff from the contract help with that? No one is forcing us to use anything in the internals!? I feel like I'm missing something here.

which array access is here if we have to be honest

ArrayAccess was never on the Enumerable contract.

In which situation do you need this?

Frankly, in every situation. If I don't need specific methods only available on the Collection, there's no reason to force my consumers into using a Collection.

In fact, if we could reverse the clock, I would default to using lazy collections everywhere (like C# does with LINQ, where even a none-lazy list becomes lazy as soon as you call additional methods on it). But that ship has sailed.

Ps. appreciating the feedback!

Always the gentleman 🥰

[driesvints]

@JosephSilber you made some good arguments against this. I'm closing this 👍

[deleugpn]

Frankly, in every situation. If I don't need specific methods only available on the Collection, there's no reason to force my consumers into using a Collection.

Since that sentiment was not applied to Eloquent Builder & Query Builder, I'm dying to get PHP 8 so that I can start using Union Type as a workaround for the lack of interface there.