Skip to content

Commit 5e1641b

Browse files
author
Six
committed
Fix iterator key collision bug and refactored test architecture
1 parent 4c0217d commit 5e1641b

File tree

6 files changed

+168
-69
lines changed

6 files changed

+168
-69
lines changed

‎composer.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848
},
4949
"scripts": {
5050
"test": "phpunit",
51-
"stan": "phpstan analyse"
51+
"stan": "phpstan analyse",
52+
"coverage": "XDEBUG_MODE=coverage phpunit --coverage-html coverage-html --coverage-text",
53+
"coverage-text": "XDEBUG_MODE=coverage phpunit --coverage-text"
5254
}
5355
}

‎phpunit.xml.dist‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
<?xml version="1.0" encoding="UTF-8"?>
22
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3-
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/11.0/phpunit.xsd"
3+
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/12.0/phpunit.xsd"
44
bootstrap="tests/bootstrap.php">
55
<testsuites>
66
<testsuite name="tests">
77
<directory>tests</directory>
88
</testsuite>
99
</testsuites>
10+
<source>
11+
<include>
12+
<directory suffix=".php">src</directory>
13+
</include>
14+
</source>
1015
</phpunit>

‎src/AbstractStrategy.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ abstract protected function next(array $elements, int $i): array;
6161
* @param list<T> $result
6262
* @return iterable<int, list<T>>
6363
*/
64-
private function generate(array $elements, int $slot = 0, array &$result = []): iterable
64+
final protected function generate(array $elements, int $slot = 0, array &$result = [], int &$index = 0): iterable
6565
{
6666
$nextSlot = $slot + 1;
6767

@@ -71,9 +71,9 @@ private function generate(array $elements, int $slot = 0, array &$result = []):
7171
$result[$slot] = $element;
7272

7373
if ($nextSlot < $this->k) {
74-
yield from $this->generate($this->next($elements, $i), $nextSlot, $result);
74+
yield from $this->generate($this->next($elements, $i), $nextSlot, $result, $index);
7575
} else {
76-
yield $result;
76+
yield $index++ => $result;
7777
}
7878
}
7979
}

‎src/Combinatorics.php‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@
77

88
final readonly class Combinatorics
99
{
10-
private function __construct()
11-
{
12-
}
13-
1410
/**
1511
* @template U
1612
* @param array<array-key, U> $elements

‎tests/AbstractStrategyTest.php‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Blibio\Combinatorics\Test;
5+
6+
use Blibio\Combinatorics\AbstractStrategy;
7+
use InvalidArgumentException;
8+
use Override;
9+
use PHPUnit\Framework\TestCase;
10+
11+
/**
12+
* Tests for AbstractStrategy algorithmic and behavioral concerns
13+
* These tests apply to all concrete strategy implementations
14+
*/
15+
final class AbstractStrategyTest extends TestCase
16+
{
17+
/**
18+
* @template U
19+
* @param array<array-key, U> $elements
20+
* @param int<1, max> $k
21+
* @return TestableStrategy<U>
22+
*/
23+
private function createTestableStrategy(array $elements, int $k): TestableStrategy
24+
{
25+
return new TestableStrategy($elements, $k);
26+
}
27+
28+
public function testGenerateProducesExpectedResults(): void
29+
{
30+
$strategy = $this->createTestableStrategy(['A', 'B', 'C'], 2);
31+
32+
$results = [];
33+
foreach ($strategy as $combo) {
34+
$results[] = $combo;
35+
}
36+
37+
$expected = [
38+
['A', 'B'],
39+
['A', 'C'],
40+
['B', 'C'],
41+
];
42+
43+
self::assertEquals($expected, $results);
44+
self::assertCount(3, $results);
45+
}
46+
47+
// === Validation Tests (Base Class Logic) ===
48+
49+
public function testThrowsOnEmptyArray(): void
50+
{
51+
$this->expectException(InvalidArgumentException::class);
52+
$this->expectExceptionMessage('Cannot generate combinations/permutations from empty array.');
53+
54+
$this->createTestableStrategy([], 1);
55+
}
56+
57+
public function testThrowsOnNumLessThanZero(): void
58+
{
59+
$this->expectException(InvalidArgumentException::class);
60+
$this->expectExceptionMessage('$k must be greater than zero, got: -1');
61+
62+
/** @phpstan-ignore argument.type */
63+
$this->createTestableStrategy(['A'], -1);
64+
}
65+
66+
// === Algorithmic/Behavioral Tests ===
67+
68+
public function testIteratorReusability(): void
69+
{
70+
$strategy = $this->createTestableStrategy(['A', 'B'], 1);
71+
72+
// First iteration
73+
$firstPass = [];
74+
foreach ($strategy as $combo) {
75+
$firstPass[] = $combo;
76+
}
77+
78+
// Second iteration on iterator
79+
$secondPass = [];
80+
foreach ($strategy as $combo) {
81+
$secondPass[] = $combo;
82+
}
83+
84+
self::assertEquals($firstPass, $secondPass);
85+
self::assertEquals([['A'], ['B']], $firstPass);
86+
}
87+
88+
public function testDuplicateElementsAreTreatedAsDistinct(): void
89+
{
90+
// Each array position is treated as a distinct identity
91+
$strategy = $this->createTestableStrategy(['A', 'A', 'B'], 2);
92+
93+
$results = [];
94+
foreach ($strategy as $combo) {
95+
$results[] = $combo;
96+
}
97+
98+
// Should get 3 combinations: first-A+second-A, first-A+B, second-A+B
99+
$expected = [
100+
['A', 'A'], // position 0 + position 1
101+
['A', 'B'], // position 0 + position 2
102+
['A', 'B'], // position 1 + position 2
103+
];
104+
105+
self::assertEquals($expected, $results);
106+
self::assertCount(3, $results);
107+
}
108+
109+
public function testIteratorToArrayPreservesAllResults(): void
110+
{
111+
$strategy = $this->createTestableStrategy(['A', 'B', 'C'], 2);
112+
113+
// Using iterator_to_array with preserve_keys = true should preserve all results
114+
$arrayResults = iterator_to_array($strategy, true);
115+
116+
// Expected: 3 combinations
117+
$expected = [
118+
['A', 'B'],
119+
['A', 'C'],
120+
['B', 'C'],
121+
];
122+
123+
// This should have all 3 results, not just the last one
124+
self::assertCount(3, $arrayResults);
125+
self::assertEquals($expected, array_values($arrayResults));
126+
}
127+
}
128+
129+
/**
130+
* Simple mock implementation for testing AbstractStrategy behavior.
131+
* Uses basic "combination without repetition" logic for simplicity.
132+
*
133+
* @template T
134+
* @extends AbstractStrategy<T>
135+
*/
136+
final readonly class TestableStrategy extends AbstractStrategy
137+
{
138+
#[Override]
139+
protected function next(array $elements, int $i): array
140+
{
141+
return array_slice($elements, $i + 1);
142+
}
143+
144+
#[Override]
145+
public function count(): int
146+
{
147+
// Simple combination formula for testing
148+
/** @var int<0, max> */
149+
return gmp_intval(
150+
gmp_div(
151+
gmp_fact($this->n),
152+
gmp_mul(gmp_fact($this->k), gmp_fact(gmp_sub($this->n, $this->k)))
153+
)
154+
);
155+
}
156+
}

‎tests/Combination/WithoutRepetitionTest.php‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,12 @@
44
namespace Blibio\Combinatorics\Test\Combination;
55

66
use Blibio\Combinatorics\Combinatorics;
7-
use Countable;
87
use InvalidArgumentException;
98
use PHPUnit\Framework\Attributes\DataProvider;
109
use PHPUnit\Framework\TestCase;
11-
use Traversable;
1210

1311
final class WithoutRepetitionTest extends TestCase
1412
{
15-
public function testThrowsOnEmptyArray(): void
16-
{
17-
$this->expectException(InvalidArgumentException::class);
18-
$this->expectExceptionMessage('Cannot generate combinations/permutations from empty array.');
19-
20-
Combinatorics::combinationsWithoutRepetition([], 1);
21-
}
22-
23-
public function testThrowsOnNumLessThanZero(): void
24-
{
25-
$this->expectException(InvalidArgumentException::class);
26-
$this->expectExceptionMessage('$k must be greater than zero, got: -1');
27-
28-
/** @phpstan-ignore argument.type */
29-
Combinatorics::combinationsWithoutRepetition(['A'], -1);
30-
}
31-
3213
public function testThrowsOnNumGreaterThanElements(): void
3314
{
3415
$this->expectException(InvalidArgumentException::class);
@@ -66,47 +47,6 @@ public function testCounts(array $elements, int $k, array $expected): void
6647
self::assertCount(count($expected), $uut);
6748
}
6849

69-
public function testIteratorReusability(): void
70-
{
71-
$uut = Combinatorics::combinationsWithoutRepetition(['A', 'B'], 1);
72-
73-
// First iteration
74-
$firstPass = [];
75-
foreach ($uut as $combo) {
76-
$firstPass[] = $combo;
77-
}
78-
79-
// Second iteration on same object
80-
$secondPass = [];
81-
foreach ($uut as $combo) {
82-
$secondPass[] = $combo;
83-
}
84-
85-
self::assertEquals($firstPass, $secondPass);
86-
self::assertEquals([['A'], ['B']], $firstPass);
87-
}
88-
89-
public function testDuplicateElementsAreTreatedAsDistinct(): void
90-
{
91-
// Each array position is treated as a distinct identity
92-
$uut = Combinatorics::combinationsWithoutRepetition(['A', 'A', 'B'], 2);
93-
94-
$results = [];
95-
foreach ($uut as $combo) {
96-
$results[] = $combo;
97-
}
98-
99-
// Should get 3 combinations: first-A+second-A, first-A+B, second-A+B
100-
$expected = [
101-
['A', 'A'], // position 0 + position 1
102-
['A', 'B'], // position 0 + position 2
103-
['A', 'B'], // position 1 + position 2
104-
];
105-
106-
self::assertEquals($expected, $results);
107-
self::assertCount(3, $results);
108-
}
109-
11050
/** @return array<array-key, mixed> */
11151
public static function results(): array
11252
{

0 commit comments

Comments
 (0)