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.

Sign up for GitHub

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

Support multidimensional and associative arrays when canonicalize #108

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Support multidimensional and associative arrays when canonicalize #108

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b1cbe3
feature/support-multidimensional-and-associative-arrays-when-canonica…
alexndlm Aug 14, 2023
501f687
- refactoring
alexndlm Aug 14, 2023
bc39224
- refactoring
alexndlm Oct 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions src/ArrayComparator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use function array_key_exists;
use function assert;
use function is_array;
use function ksort;
use function sort;
use function sprintf;
use function str_replace;
Expand Down Expand Up @@ -39,8 +40,8 @@ public function assertEquals(mixed $expected, mixed $actual, float $delta = 0.0,
assert(is_array($actual));

if ($canonicalize) {
sort($expected);
sort($actual);
$this->canonicalize($expected);
$this->canonicalize($actual);
}

$remaining = $actual;
Expand Down Expand Up @@ -124,4 +125,33 @@ private function indent(string $lines): string
{
return trim(str_replace("\n", "\n ", $lines));
}

private function canonicalize(array &$array): void
{
if ($this->isIndexedArray($array)) {
sort($array);
} else {
ksort($array);
}

foreach ($array as &$element) {
if (is_array($element)) {
Copy link

Choose a reason for hiding this comment

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

If an element of the parent $array is an array, won't it get passed to the ArrayComparator@assertEquals() method? It seems like we wouldn't need to recursively sort here if that's true.

Copy link
Author

Choose a reason for hiding this comment

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

Recursion is needed. After all, nesting can be as big as you want, for example:

$a = [
    'k' => [
        'h' => [
            'n' => [
                ....
            ],
        ],
    ],
];

Copy link

Choose a reason for hiding this comment

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

Yes, but assertEquals() is going to loop through each sub-element of the array and then call Comparator@assertEquals() on that.

If the sub-element is an array, that means it's going to get passed to ArrayComparator@canonicalize() again.

So you're sorting everything, and then sorting it again after creating the factory on lines 70-71.

Do I have that wrong?

Copy link
Author

Choose a reason for hiding this comment

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

I finally got it. It's brilliant!

$this->canonicalize($element);
}
}
}

private function isIndexedArray(array $array): bool
sebastianbergmann marked this conversation as resolved.
Show resolved Hide resolved
{
$expectedKey = 0;

foreach ($array as $key => $value) {
if ($key !== $expectedKey) {
return false;
}
$expectedKey++;
}

return true;
}
}
12 changes: 12 additions & 0 deletions tests/ArrayComparatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ public static function assertEqualsSucceedsProvider()
['true'],
[true],
],
[
['a', 'b' => [1, 2]],
['b' => [2, 1], 'a'],
0,
true,
],
];
}

Expand Down Expand Up @@ -115,6 +121,12 @@ public static function assertEqualsFailsProvider()
['false'],
[false],
],
[
['a', 'b' => [1, 2]],
['b' => [2, 1], 'a', 'c' => 3],
0,
true,
],
];
}

Expand Down