Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.2] Support/Array : add Arr class #4620

Merged
merged 1 commit into from Jul 18, 2014
Merged

[4.2] Support/Array : add Arr class #4620

merged 1 commit into from Jul 18, 2014

Conversation

lucasmichot
Copy link
Contributor

Move array_* functions from helpers.php to new Arr class.
(just following the way it has been done for string functions in Str class)

@Anahkiasen
Copy link
Contributor

👍

@crynobone
Copy link
Member

#3537 was turned down. Might want to better describe the need for this.

@Anahkiasen
Copy link
Contributor

It was closed but honestly I'm not sure why as this had been agreed upon previously.

@barryvdh
Copy link
Contributor

barryvdh commented Jun 9, 2014

👍
But perhaps import the class with use Illuminate\Support\Arr;, so you can use the short class in the helpers?

@lucasmichot
Copy link
Contributor Author

Good catch @barryvdh , squashed and commited

@barryvdh
Copy link
Contributor

barryvdh commented Jun 9, 2014

Oh I see now that the Str classes also didn't do that (don't know why).

But cool, I don't think the Laravel core should be dependent on all those helpers.

@lucasmichot
Copy link
Contributor Author

Just opened a PR for that @barryvdh #4635 ;)

@KennedyTedesco
Copy link
Contributor

I really like this. 👍

@JosephSilber
Copy link
Member

@crynobone one advantage to using a class would be for splitting a complex function into multiple methods.

See this thread for an example.

* @param \CLosure $callback
* @return array
*/
static function build($array, \CLosure $callback)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLosure typo there.

@taylorotwell
Copy link
Member

Can you look at the merge conflicts on this?

@lucasmichot
Copy link
Contributor Author

Done and squashed @taylorotwell

@GrahamCampbell
Copy link
Member

That commit name is pretty bad...

@lucasmichot lucasmichot changed the title [4.2] Support : add Arr class [4.2] Support/Array : add Arr class Jul 1, 2014
@lucasmichot
Copy link
Contributor Author

@taylorotwell , is everything good for this PR ?

taylorotwell added a commit that referenced this pull request Jul 18, 2014
[4.2] Support/Array : add Arr class
@taylorotwell taylorotwell merged commit e9529ba into laravel:4.2 Jul 18, 2014
@lucasmichot lucasmichot deleted the 4.2-array-helper branch July 18, 2014 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants