Table class: refactoring group and related methods

[roberto-butti]

Consider this suggestion: https://twitter.com/marekgibney/status/1519938757893439488

$table->group('product')->transform(Operation::aggregate(['total'=>['price', 'array_sum']]));

Table::group -> Groups rows by field
Table::transform -> Transforms rows by func
Operation::aggregate -> Aggregates fields by funcs

Checklist

• implement method
• write phpdoc for the function
• write test in tests/TableTest.php
• execute composer format
• execute composer test-coverage
• check if your method has full test coverage
• add method in changelog
• update doc/table.md adding a use case of the new method
• update cheatsheet (examples/cheatsheet-table.php)

[RoadSigns]

@roberto-butti Are you still interested in this being done?

[roberto-butti]

@roberto-butti Are you still interested in this being done?

sure i created this issue for when i will have more time to focus on this. but if you want to work on it... you are more than welcome !!!

[RoadSigns]

@roberto-butti
So I've been taking a look into this and I have a few questions.

Dropping the Extension of Arr

Do we want to remove the Arr extension from the Table class so the public methods are what we want to control since extending the Arr opens it up to a pile of public methods that might not work on the underlying array that is being used in Table since that seems to be more focused on associative arrays?

Expectation of group()

So for group('product') do you want it to work like SQL where we group all rows that are the same and only keep the first one if another one turns up?

So the following would go from:

['product' => 'Desk', 'price' => 200, 'active' => true],
['product' => 'Chair', 'price' => 100, 'active' => true],
['product' => 'Door', 'price' => 300, 'active' => false],
['product' => 'Bookcase', 'price' => 150, 'active' => true],
['product' => 'Door', 'price' => 100, 'active' => true],

To:

['product' => 'Desk', 'price' => 200, 'active' => true],
['product' => 'Chair', 'price' => 100, 'active' => true],
['product' => 'Bookcase', 'price' => 150, 'active' => true],

Transform using Closure instead

For the transform() I was thinking of having it so we have transform(string|int $newColumn, Closure $closure): self that way we can just let the user put whatever functions they want in and it'll just run across all of the table for each row and then return a new column with whatever name they wanted.

This would allow us to achieve what was requested in the twitter via:

// ['product' => 'Desk', 'price' => 200, 'active' => true],
// ['product' => 'Chair', 'price' => 100, 'active' => true],
// ['product' => 'Door', 'price' => 300, 'active' => false],
// ['product' => 'Bookcase', 'price' => 150, 'active' => true],
// ['product' => 'Door', 'price' => 100, 'active' => true],

$table
	->group('product') // Returns a Table object with the rows grouped by the column so we would lose all of the information
	->transform('product', static function($row) {
		return strtolower($row['product']) 
	})
	->arr();

// ['product' => 'desk', 'price' => 200, 'active' => true],
// ['product' => 'chair', 'price' => 100, 'active' => true],
// ['product' => 'bookcase', 'price' => 150, 'active' => true],

[roberto-butti]

Thank you @RoadSigns , i will take a look deeply the next day.
Thank you for all your feedback!!!

[roberto-butti]

For group('product') what do you think about something:
From:

['product' => 'Desk', 'price' => 200, 'active' => true],
['product' => 'Chair', 'price' => 100, 'active' => true],
['product' => 'Door', 'price' => 300, 'active' => false],
['product' => 'Bookcase', 'price' => 150, 'active' => true],
['product' => 'Door', 'price' => 100, 'active' => true],

you obtain something by deafult like the fields in product-data are the fields specified in select():

['product' => 'Desk', 'product-data' => [ ['price' => 200, 'active' => true] ],
['product' => 'Chair', 'product-data' => [ ['price' => 100, 'active' => true] ],
['product' => 'Door', 'product-data' => [  ['price' => 300, 'active' => false], ['price' => 100, 'active' => true] ]
...

if no select is specified you will return just

['product' => 'Desk'],
['product' => 'Chair'],
['product' => 'Door'],
...

what do you think @RoadSigns ?

[RoadSigns]

@roberto-butti

Since we will be returning a Table object back which is mostly a two dimensional array this will get confusing now introducing a third dimension via the product-data array. Plus it might now allow us to do things like $table->group('product')->orderBy('product');

[roberto-butti]

@RoadSigns thank you for the feedback. So, i think that the solution that works like SQL that you suggested is totally fine!!!

Roberto

[RoadSigns]

@roberto-butti

Group By implementation

So I've created a merge request to tackle some of this. #50
This will allow us to group all of the information like how SQL does this and we can change methods on this such as:

$table->where('active', '=', true)->orderBy('price', 'asc')->groupBy('product');
// Expected Result:
//  ['product' => 'Chair', 'price' => 100, 'active' => true],
//  ['product' => 'Door', 'price' => 100, 'active' => true],
//  ['product' => 'Bookcase', 'price' => 150, 'active' => true],
//  ['product' => 'Desk', 'price' => 200, 'active' => true],

This will allow us to get the cheapest price for all of the specific products.

Additional Refactoring

Since we currently can't guarantee what is inside of the table. I was going to make a PR after this that changes the Table class.

This means that it goes from Table extends Arr to just Table this means that we will have to implement all of the methods needed. The reason for this is since we can then have Table have an array of Arr as the property so that way we will have all of the methods for the row on the object being returned.

We can then start leaning on the fact we know what we are looping over when it comes to the implementation of methods like sum(), avg() and count() on "columns" inside of the table.

That way we can start doing things like:

$table->where('active', '=', true)->avg('price');

[RoadSigns]

@roberto-butti Are you happy that this has been achieved or are there some missing features from this suggestion?

[roberto-butti]

@RoadSigns, I think you did a great job on this, already merged.
So I think that I can close this issue. If you agree, I will open one specific to improve the documentation (readme and example, because these functionalities are great)

[RoadSigns]

@roberto-butti
That sounds perfectly fine to me, I'm happy to be assigned to the documentation since I did all the fun part of the work, so it's only fair I write it up!