pomm-project / pomm-symfony-bridge

Pomm2 shared elements for profiler between Silex and Symfony.
8 stars 17 forks source link

Data array should be properly initialized on reset #38

Open a-lutz opened 6 years ago

a-lutz commented 6 years ago

In Behat tests I had some nasty notices like "Notice: Undefined index: time in vendor/pomm-project/pomm-symfony-bridge/sources/lib/DatabaseDataCollector.php line 62"

Data reset should set back data array to construct values, not an empty array.

sanpii commented 6 years ago

Thank you.

I have just a comment about design: I think it’s a good idea to create a new Data class to avoid method with side-effet. Then you can write $this->data = new Data() instead of $this->initData(). What do you think?

a-lutz commented 6 years ago

Hello, Could it be used somewhere else ? It looks more like a purely internal array to me (actually just realize it's never declared)

sanpii commented 6 years ago

Could it be used somewhere else ?

No, just here.

It looks more like a purely internal array to me

Yes, but having something more strict than an array seens a good idea.

At least, can you change the initData method to return the data instead of modify the property directly.

a-lutz commented 6 years ago

That I can :)

a-lutz commented 6 years ago

Thank you Sanpii :) Do you have an idea for the date of 2.5.1 ?

sanpii commented 6 years ago

When your PR will be merged 😋 Maybe it’s also a good idea to fix #34