Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

Add a dd function to the collection class #117

Closed
freekmurze opened this issue Jun 16, 2016 · 11 comments
Closed

Add a dd function to the collection class #117

freekmurze opened this issue Jun 16, 2016 · 11 comments

Comments

@freekmurze
Copy link

freekmurze commented Jun 16, 2016

Image you are debugging a collection chain. You want to see what the values are after the map function in the example below. To do that you could wrap the code starting from the collection call until right after the map function in a dd function.

dd(collect($items)
  ->filter(function() { 
     ... 
   })
   ->unique(function() { 
      ... 
   })
   ->map(function() {
     ... 
   }))
   ->sortBy(function() { 
      ...
   });

It's a bit of work to debug like this. You have to insert code a the beginning of the collection and an extra closing parenthesis after the map function. To my eyes this isn't very readable. It's also a bit of a hassle to remove the dd statement, that closing parenthesis is easily forgotten. I'm exaggerating the problems a bit, but debugging like this becomes tiresome real quick.

If we'd add a dd function to Collection the workflow can be drastically improved. This is how the function would look like

public function dd() 
{
   dd($this);
}

Nice 'n' simple.

This is how it can be used:

collect($items)
  ->filter(function() { 
     ... 
   })
   ->unique(function() { 
      ... 
   })
   ->map(function() {
     ... 
   })
   ->dd()
   ->sortBy(function() { 
      ...
   });

To see the values after a particular step in the chain you simply have to add one line of code. It's perfectly readable. After you've done your debugging there's only one line to remove. Using for example PHPStorm's alt+shift+arrow-up and alt+shift+arrow-down shortcuts the ->dd() line can be easily moved to the previous or next part of the collection chain.

What are your thoughts on adding this to Collection? Handy? Or feature creep? I've not ran any statistics but pretty sure a lot of users will be happy with this 😄

If the response is positive I'll send a PR.

@franzliedke
Copy link

What about the more generic tap? (That would allow to pass a closure that can do anything - such as dd with the current array of items - without having to return it like with map.)

@freekmurze
Copy link
Author

I've already submitted a PR with the functionality you're describing. I've named it pipe, like in @adamwathan 's excellent book on collections.

If that PR get's accepted you can do this:

...
->pipe('dd');
...

imho because dd'ing a collection is so handy and you'll probably do it a lot, it could deserve it's on short function.

@rossbearman
Copy link

rossbearman commented Jun 16, 2016

I'd certainly be in favour of it. Even with xdebug easily available, I still end up dd()ing data a lot and just dropping a new function into the chain is less disruptive than having to wrap and unwrap parts of it as you go.

The only real negatives I can see are that it's a purely development focused tool that introduces a new side effect to Collections (that they can dump their data directly to output) and the functionality can be very quickly implemented with a macro. But I'm not convinced that that's a good argument not to include something that essentially requires no maintenance due to it's simplicity and provides a good quality of life improvement to working with Collection pipelines.

@JosephSilber
Copy link
Member

The general idea sounds good, although if the pipe PR gets merged this is of less importance.

If we do do this, I think it should dd($this->all()) or maybe even dd($this->toArray()).

@sebastiandedeyne
Copy link

@JosephSilber I think I have a slight favor for dd($this). It could be useful to see the object type you're working with, like when you're not sure whether you have an eloquent or a base collection.

@JosephSilber
Copy link
Member

JosephSilber commented Jun 16, 2016

@sebastiandedeyne you could still do it the old way. Just saying that 99% of the time ->all() or toArray() would be what you want.

@rossbearman
Copy link

rossbearman commented Jun 16, 2016

Would it be worth optionally supporting passing a callback in to the dd() method of the Collection class to allow more complex output with the current Collection? Something like:

    /**
     * @param  callable  $callback
     * @return static
     */
    public function dd(callable $callback = null)
    {
        if(is_callable($callback)) {
            dd($callback($this));
        }

        dd($this->all());
    }

@JosephSilber
Copy link
Member

JosephSilber commented Jun 16, 2016 via email

@garygreen
Copy link

Trouble with doing this is you could say the same about models and other things. If I've done a big collection this like that I would have it in a variable anyway so could just do dd($collection);

@freekmurze
Copy link
Author

Meanwhile I've created a package containing the dd macro along with a few
others that don't really belong in the core.

https://github.com/spatie/laravel-collection-macros

On Friday, 19 August 2016, Gary Green notifications@github.com wrote:

Trouble with doing this is you could say the same about models and other
things. If I've done a big collection this like that I would have it in a
variable anyway so could just do dd($collection);


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#117 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAdiDdv-ADJpXfbyvCAhHp0l9w4cf2Xkks5qhZAVgaJpZM4I3Hn0
.

Freek Van der Herten
https://spatie.be
+32 495 84 27 91

@taylorotwell
Copy link
Member

Closing this since we have pipe I guess.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants