laravel / ideas

Issues board used for Laravel internals discussions.
939 stars 28 forks source link

[Proposal] Standardise 'has' method behaviour throughout the framework #199

Open ntzm opened 8 years ago

ntzm commented 8 years ago

The problem

There is some inconsistency in the framework regarding checking for things using has methods: there are some which allow for multiple checks in one call, and others than only allow for single checks, as detailed below.

Can check multiple

The following functions can take arguments as a single key, and array of keys, or multiple arguments. They return true if all keys exist.

These functions can take a single key or an array of keys, due to them using Arr::has(), but it is not advertised in the PHPDoc. They return true if all keys exist.

The same as above, but they return true if at least one of the keys exist.

And these can only check single keys.

This may not be an extensive list, but I believe I got most of them. Some of these also may be in the wrong category, as I only did a quick scan of each function to figure out what it did.

Proposal

Here is my proposal:

Obviously these are just ideas, feel free to add and critique, I haven't really thought a whole lot about it.

tristanbailey commented 8 years ago

Would you not also think of adding standard naming where existing methods exist and passing though the old ones so they could be depreciated.

To understand if it is the standard use or the standard naming as well you are interested in, as I think you have a good point.

ntzm commented 8 years ago

@tristanbailey Sorry I don't quite understand what you mean

tristanbailey commented 8 years ago

@ntzm Hi, you have made a comment to unify the has() statements but at the moment your fork has some changes of function but not renaming the for example hasQueued() to has() (I understand you are still working so could change)

To do this and keep backwards compatibility you could add pass throughs, allowing for migration.

has($key) {
    return self:: hasQueued($key);
}

So was asking is the No 1 aim to have all methods behaviour the same or to be named the same also, to point to this fact?

tristanbailey commented 8 years ago

side note: Some methods like Illuminate\Foundation\Http\Kernel::hasMiddleware() might not be appropriate to call Kernel::has() on, so maybe as an addition to your proposal an optional param of 'type' might be needed

// idea code 
public function has($key, $type='Middleware') {
    // ... if type middleware 
    // ... if type auth
    // ...
}
ntzm commented 8 years ago

@tristanbailey I didn't mean we should rename the methods, just allow them to be passed multiple arguments. Sorry for the confusion

troccoli commented 6 years ago

@ntzm, I don't know how far you've got with it and I would like to help if I can.

I found this proposal because I was tired of writing something like if (!$collection->has('key')), and I wanted to contribute to make Laravel better.

So, I would propose two more methods: