Skip to content

Commit 9019523

Browse files
author
MarkBaker
committed
Additional unit testing
And a quick bugfix for cell ranges applied to both sort functions and to FILTER()
1 parent 7f00049 commit 9019523

File tree

3 files changed

+126
-39
lines changed

3 files changed

+126
-39
lines changed

‎src/PhpSpreadsheet/Calculation/LookupRef/Filter.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ public static function filter($lookupArray, $matchArray, $ifEmpty = null)
1919
return ExcelError::VALUE();
2020
}
2121

22+
$matchArray = self::enumerateArrayKeys($matchArray);
23+
2224
$result = (Matrix::isColumnVector($matchArray))
2325
? self::filterByRow($lookupArray, $matchArray)
2426
: self::filterByColumn($lookupArray, $matchArray);
@@ -30,6 +32,20 @@ public static function filter($lookupArray, $matchArray, $ifEmpty = null)
3032
return array_values($result);
3133
}
3234

35+
private static function enumerateArrayKeys(array $sortArray): array
36+
{
37+
array_walk(
38+
$sortArray,
39+
function (&$columns): void {
40+
if (is_array($columns)) {
41+
$columns = array_values($columns);
42+
}
43+
}
44+
);
45+
46+
return array_values($sortArray);
47+
}
48+
3349
private static function filterByRow(array $lookupArray, array $matchArray): array
3450
{
3551
$matchArray = array_values(array_column($matchArray, 0));

‎src/PhpSpreadsheet/Calculation/LookupRef/Sort.php‎

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,23 @@ class Sort extends LookupRefValidations
2121
* Both $sortIndex and $sortOrder can be arrays, to provide multi-level sorting.
2222
*
2323
* @param mixed $sortArray The range of cells being sorted
24-
* @param mixed $sortIndex Whether the uniqueness should be determined by row (the default) or by column
24+
* @param mixed $sortIndex The column or row number within the sortArray to sort on
2525
* @param mixed $sortOrder Flag indicating whether to sort ascending or descending
2626
* Ascending = 1 (self::ORDER_ASCENDING)
2727
* Descending = -1 (self::ORDER_DESCENDING)
2828
* @param mixed $byColumn Whether the sort should be determined by row (the default) or by column
2929
*
3030
* @return mixed The sorted values from the sort range
3131
*/
32-
public static function sort($sortArray, $sortIndex = [1], $sortOrder = self::ORDER_ASCENDING, $byColumn = false)
32+
public static function sort($sortArray, $sortIndex = 1, $sortOrder = self::ORDER_ASCENDING, $byColumn = false)
3333
{
3434
if (!is_array($sortArray)) {
3535
// Scalars are always returned "as is"
3636
return $sortArray;
3737
}
3838

39+
$sortArray = self::enumerateArrayKeys($sortArray);
40+
3941
$byColumn = (bool) $byColumn;
4042
$lookupIndexSize = $byColumn ? count($sortArray) : count($sortArray[0]);
4143

@@ -68,6 +70,12 @@ public static function sort($sortArray, $sortIndex = [1], $sortOrder = self::ORD
6870
*
6971
* @param mixed $sortArray The range of cells being sorted
7072
* @param mixed $args
73+
* At least one additional argument must be provided, The vector or range to sort on
74+
* After that, arguments are passed as pairs:
75+
* sort order: ascending or descending
76+
* Ascending = 1 (self::ORDER_ASCENDING)
77+
* Descending = -1 (self::ORDER_DESCENDING)
78+
* additional arrays or ranges for multi-level sorting
7179
*
7280
* @return mixed The sorted values from the sort range
7381
*/
@@ -78,6 +86,8 @@ public static function sortBy($sortArray, ...$args)
7886
return $sortArray;
7987
}
8088

89+
$sortArray = self::enumerateArrayKeys($sortArray);
90+
8191
$lookupArraySize = count($sortArray);
8292
$argumentCount = count($args);
8393

@@ -94,19 +104,33 @@ public static function sortBy($sortArray, ...$args)
94104
return self::processSortBy($sortArray, $sortBy, $sortOrder);
95105
}
96106

107+
private static function enumerateArrayKeys(array $sortArray): array
108+
{
109+
array_walk(
110+
$sortArray,
111+
function (&$columns): void {
112+
if (is_array($columns)) {
113+
$columns = array_values($columns);
114+
}
115+
}
116+
);
117+
118+
return array_values($sortArray);
119+
}
120+
97121
/**
98122
* @param mixed $sortIndex
99123
* @param mixed $sortOrder
100124
*/
101-
private static function validateScalarArgumentsForSort(&$sortIndex, &$sortOrder, int $lookupIndexSize): void
125+
private static function validateScalarArgumentsForSort(&$sortIndex, &$sortOrder, int $sortArraySize): void
102126
{
103127
if (is_array($sortIndex) || is_array($sortOrder)) {
104128
throw new Exception(ExcelError::VALUE());
105129
}
106130

107131
$sortIndex = self::validatePositiveInt($sortIndex, false);
108132

109-
if ($sortIndex > $lookupIndexSize) {
133+
if ($sortIndex > $sortArraySize) {
110134
throw new Exception(ExcelError::VALUE());
111135
}
112136

@@ -116,15 +140,15 @@ private static function validateScalarArgumentsForSort(&$sortIndex, &$sortOrder,
116140
/**
117141
* @param mixed $sortVector
118142
*/
119-
private static function validateSortVector($sortVector, int $lookupArraySize): array
143+
private static function validateSortVector($sortVector, int $sortArraySize): array
120144
{
121145
if (!is_array($sortVector)) {
122146
throw new Exception(ExcelError::VALUE());
123147
}
124148

125149
// It doesn't matter if it's a row or a column vectors, it works either way
126150
$sortVector = Functions::flattenArray($sortVector);
127-
if (count($sortVector) !== $lookupArraySize) {
151+
if (count($sortVector) !== $sortArraySize) {
128152
throw new Exception(ExcelError::VALUE());
129153
}
130154

@@ -148,14 +172,14 @@ private static function validateSortOrder($sortOrder): int
148172
* @param array $sortIndex
149173
* @param mixed $sortOrder
150174
*/
151-
private static function validateArrayArgumentsForSort(&$sortIndex, &$sortOrder, int $lookupIndexSize): void
175+
private static function validateArrayArgumentsForSort(&$sortIndex, &$sortOrder, int $sortArraySize): void
152176
{
153177
// It doesn't matter if they're row or column vectors, it works either way
154178
$sortIndex = Functions::flattenArray($sortIndex);
155179
$sortOrder = Functions::flattenArray($sortOrder);
156180

157181
if (
158-
count($sortOrder) === 0 || count($sortOrder) > $lookupIndexSize ||
182+
count($sortOrder) === 0 || count($sortOrder) > $sortArraySize ||
159183
(count($sortOrder) > count($sortIndex))
160184
) {
161185
throw new Exception(ExcelError::VALUE());
@@ -170,7 +194,7 @@ private static function validateArrayArgumentsForSort(&$sortIndex, &$sortOrder,
170194
}
171195

172196
foreach ($sortIndex as $key => &$value) {
173-
self::validateScalarArgumentsForSort($value, $sortOrder[$key], $lookupIndexSize);
197+
self::validateScalarArgumentsForSort($value, $sortOrder[$key], $sortArraySize);
174198
}
175199
}
176200

@@ -195,7 +219,7 @@ function ($value) {
195219
* @param array[] $sortIndex
196220
* @param int[] $sortOrder
197221
*/
198-
private static function processSortBy(array $lookupArray, array $sortIndex, $sortOrder): array
222+
private static function processSortBy(array $sortArray, array $sortIndex, $sortOrder): array
199223
{
200224
$sortArguments = [];
201225
$sortData = [];
@@ -204,32 +228,32 @@ private static function processSortBy(array $lookupArray, array $sortIndex, $sor
204228
$sortArguments[] = self::prepareSortVectorValues($sortValues);
205229
$sortArguments[] = $sortOrder[$index] === self::ORDER_ASCENDING ? SORT_ASC : SORT_DESC;
206230
}
207-
$sortArguments = self::applyPHP7Patch($lookupArray, $sortArguments);
231+
$sortArguments = self::applyPHP7Patch($sortArray, $sortArguments);
208232

209233
$sortVector = self::executeVectorSortQuery($sortData, $sortArguments);
210234

211-
return self::sortLookupArrayFromVector($lookupArray, $sortVector);
235+
return self::sortLookupArrayFromVector($sortArray, $sortVector);
212236
}
213237

214238
/**
215239
* @param int[] $sortIndex
216240
* @param int[] $sortOrder
217241
*/
218-
private static function sortByRow(array $lookupArray, array $sortIndex, array $sortOrder): array
242+
private static function sortByRow(array $sortArray, array $sortIndex, array $sortOrder): array
219243
{
220-
$sortVector = self::buildVectorForSort($lookupArray, $sortIndex, $sortOrder);
244+
$sortVector = self::buildVectorForSort($sortArray, $sortIndex, $sortOrder);
221245

222-
return self::sortLookupArrayFromVector($lookupArray, $sortVector);
246+
return self::sortLookupArrayFromVector($sortArray, $sortVector);
223247
}
224248

225249
/**
226250
* @param int[] $sortIndex
227251
* @param int[] $sortOrder
228252
*/
229-
private static function sortByColumn(array $lookupArray, array $sortIndex, array $sortOrder): array
253+
private static function sortByColumn(array $sortArray, array $sortIndex, array $sortOrder): array
230254
{
231-
$lookupArray = Matrix::transpose($lookupArray);
232-
$result = self::sortByRow($lookupArray, $sortIndex, $sortOrder);
255+
$sortArray = Matrix::transpose($sortArray);
256+
$result = self::sortByRow($sortArray, $sortIndex, $sortOrder);
233257

234258
return Matrix::transpose($result);
235259
}
@@ -238,17 +262,17 @@ private static function sortByColumn(array $lookupArray, array $sortIndex, array
238262
* @param int[] $sortIndex
239263
* @param int[] $sortOrder
240264
*/
241-
private static function buildVectorForSort(array $lookupArray, array $sortIndex, array $sortOrder): array
265+
private static function buildVectorForSort(array $sortArray, array $sortIndex, array $sortOrder): array
242266
{
243267
$sortArguments = [];
244268
$sortData = [];
245269
foreach ($sortIndex as $index => $sortIndexValue) {
246-
$sortValues = array_column($lookupArray, $sortIndexValue - 1);
270+
$sortValues = array_column($sortArray, $sortIndexValue - 1);
247271
$sortData[] = $sortValues;
248272
$sortArguments[] = self::prepareSortVectorValues($sortValues);
249273
$sortArguments[] = $sortOrder[$index] === self::ORDER_ASCENDING ? SORT_ASC : SORT_DESC;
250274
}
251-
$sortArguments = self::applyPHP7Patch($lookupArray, $sortArguments);
275+
$sortArguments = self::applyPHP7Patch($sortArray, $sortArguments);
252276

253277
$sortData = self::executeVectorSortQuery($sortData, $sortArguments);
254278

@@ -279,12 +303,12 @@ private static function executeVectorSortQuery(array $sortData, array $sortArgum
279303
return $sortedData;
280304
}
281305

282-
private static function sortLookupArrayFromVector(array $lookupArray, array $sortVector): array
306+
private static function sortLookupArrayFromVector(array $sortArray, array $sortVector): array
283307
{
284308
// Building a new array in the correct (sorted) order works; but may be memory heavy for larger arrays
285309
$sortedArray = [];
286310
foreach ($sortVector as $index) {
287-
$sortedArray[] = $lookupArray[$index];
311+
$sortedArray[] = $sortArray[$index];
288312
}
289313

290314
return $sortedArray;
@@ -306,10 +330,10 @@ private static function sortLookupArrayFromVector(array $lookupArray, array $sor
306330
* MS Excel replicates the PHP 8.0.0 behaviour, retaining the original order of matching elements.
307331
* To replicate that behaviour with PHP 7, we add an extra sort based on the row index.
308332
*/
309-
private static function applyPHP7Patch(array $lookupArray, array $sortArguments): array
333+
private static function applyPHP7Patch(array $sortArray, array $sortArguments): array
310334
{
311335
if (PHP_VERSION_ID < 80000) {
312-
$sortArguments[] = range(1, count($lookupArray));
336+
$sortArguments[] = range(1, count($sortArray));
313337
$sortArguments[] = SORT_ASC;
314338
}
315339

‎tests/PhpSpreadsheetTests/Calculation/Functions/LookupRef/SortByTest.php‎

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public function testSortOnScalar(): void
1212
{
1313
$value = 'NON-ARRAY';
1414

15-
$result = Sort::sort($value, [1]);
15+
$result = Sort::sortBy($value);
1616
self::assertSame($value, $result);
1717
}
1818

@@ -45,16 +45,16 @@ public function providerSortWithScalarArgumentErrorReturns(): array
4545
/**
4646
* @dataProvider providerSortByRow
4747
*/
48-
public function testSortByRow(array $expectedResult, array $matrix, array $sortIndex, int $sortOrder = Sort::ORDER_ASCENDING): void
48+
public function testSortByRow(array $expectedResult, array $matrix, ...$args): void
4949
{
50-
$result = Sort::sortBy($matrix, $sortIndex, $sortOrder);
50+
$result = Sort::sortBy($matrix, ...$args);
5151
self::assertSame($expectedResult, $result);
5252
}
5353

5454
public function providerSortByRow(): array
5555
{
5656
return [
57-
[
57+
'Simple sort by age' => [
5858
[
5959
['Fritz', 19],
6060
['Xi', 19],
@@ -65,10 +65,10 @@ public function providerSortByRow(): array
6565
['Hector', 66],
6666
['Sal', 73],
6767
],
68-
$this->sampleDataForRow(),
69-
array_column($this->sampleDataForRow(), 1),
68+
$this->sampleDataForSimpleSort(),
69+
array_column($this->sampleDataForSimpleSort(), 1),
7070
],
71-
[
71+
'Simple sort by name' => [
7272
[
7373
['Amy', 22],
7474
['Fred', 65],
@@ -79,10 +79,10 @@ public function providerSortByRow(): array
7979
['Tom', 52],
8080
['Xi', 19],
8181
],
82-
$this->sampleDataForRow(),
83-
array_column($this->sampleDataForRow(), 0),
82+
$this->sampleDataForSimpleSort(),
83+
array_column($this->sampleDataForSimpleSort(), 0),
8484
],
85-
[
85+
'Row vector' => [
8686
[
8787
['Amy', 22],
8888
['Fred', 65],
@@ -93,10 +93,10 @@ public function providerSortByRow(): array
9393
['Tom', 52],
9494
['Xi', 19],
9595
],
96-
$this->sampleDataForRow(),
96+
$this->sampleDataForSimpleSort(),
9797
['Tom', 'Fred', 'Amy', 'Sal', 'Fritz', 'Srivan', 'Xi', 'Hector'],
9898
],
99-
[
99+
'Column vector' => [
100100
[
101101
['Amy', 22],
102102
['Fred', 65],
@@ -107,13 +107,46 @@ public function providerSortByRow(): array
107107
['Tom', 52],
108108
['Xi', 19],
109109
],
110-
$this->sampleDataForRow(),
110+
$this->sampleDataForSimpleSort(),
111111
[['Tom'], ['Fred'], ['Amy'], ['Sal'], ['Fritz'], ['Srivan'], ['Xi'], ['Hector']],
112112
],
113+
'Sort by region asc, name asc' => [
114+
[
115+
['East', 'Fritz', 19],
116+
['East', 'Tom', 52],
117+
['North', 'Amy', 22],
118+
['North', 'Xi', 19],
119+
['South', 'Hector', 66],
120+
['South', 'Sal', 73],
121+
['West', 'Fred', 65],
122+
['West', 'Srivan', 39],
123+
],
124+
$this->sampleDataForMultiSort(),
125+
array_column($this->sampleDataForMultiSort(), 0),
126+
Sort::ORDER_ASCENDING,
127+
array_column($this->sampleDataForMultiSort(), 1),
128+
],
129+
'Sort by region asc, age desc' => [
130+
[
131+
['East', 'Tom', 52],
132+
['East', 'Fritz', 19],
133+
['North', 'Amy', 22],
134+
['North', 'Xi', 19],
135+
['South', 'Sal', 73],
136+
['South', 'Hector', 66],
137+
['West', 'Fred', 65],
138+
['West', 'Srivan', 39],
139+
],
140+
$this->sampleDataForMultiSort(),
141+
array_column($this->sampleDataForMultiSort(), 0),
142+
Sort::ORDER_ASCENDING,
143+
array_column($this->sampleDataForMultiSort(), 2),
144+
Sort::ORDER_DESCENDING,
145+
],
113146
];
114147
}
115148

116-
private function sampleDataForRow(): array
149+
private function sampleDataForSimpleSort(): array
117150
{
118151
return [
119152
['Tom', 52],
@@ -126,4 +159,18 @@ private function sampleDataForRow(): array
126159
['Hector', 66],
127160
];
128161
}
162+
163+
private function sampleDataForMultiSort(): array
164+
{
165+
return [
166+
['North', 'Amy', 22],
167+
['West', 'Fred', 65],
168+
['East', 'Fritz', 19],
169+
['South', 'Hector', 66],
170+
['South', 'Sal', 73],
171+
['West', 'Srivan', 39],
172+
['East', 'Tom', 52],
173+
['North', 'Xi', 19],
174+
];
175+
}
129176
}

0 commit comments

Comments
 (0)