Skip to content

Commit a920559

Browse files
kristos80kristos80
authored
Merge pull request #1 from kristos80/refactor/code-improvements
refactor: improve code organization and maintainability
2 parents 49ae6b8 + 173ed22 commit a920559

File tree

5 files changed

+111
-141
lines changed

5 files changed

+111
-141
lines changed

‎src/CharacterPool.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Kristos80\PasswordGenerator;
5+
6+
enum CharacterPool: string {
7+
case CHARACTERS = "abcdefghijklmnopqrstuvwxyz";
8+
case NUMBERS = "0123456789";
9+
case SYMBOLS = "!@#$%^&*()-_=+[]{}|;:,.<>?";
10+
11+
public function getPool(): string {
12+
return $this->value;
13+
}
14+
}

‎src/PasswordGenerator.php‎

Lines changed: 79 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -7,35 +7,7 @@
77

88
final readonly class PasswordGenerator {
99

10-
/**
11-
*
12-
*/
13-
private const CHARACTERS = "abcdefghijklmnopqrstuvwxyz";
14-
15-
/**
16-
*
17-
*/
18-
private const NUMBERS = "0123456789";
19-
20-
/**
21-
*
22-
*/
23-
private const SYMBOLS = "!@#$%^&*()-_=+[]{}|;:,.<>?";
24-
25-
/**
26-
*
27-
*/
28-
private const POOL_CHARACTERS = "characters";
29-
30-
/**
31-
*
32-
*/
33-
private const POOL_NUMBERS = "numbers";
34-
35-
/**
36-
*
37-
*/
38-
private const POOL_SYMBOLS = "symbols";
10+
private const MAX_CONSECUTIVE_AVOIDANCE_ATTEMPTS = 5;
3911

4012
/**
4113
* @param PasswordGeneratorConfig $generatorConfig
@@ -46,102 +18,124 @@
4618
public function generate(PasswordGeneratorConfig $generatorConfig): string {
4719
$password = [];
4820

49-
$lowerCount = random_int(
50-
$generatorConfig->getLowercaseRange()->min,
51-
$generatorConfig->getLowercaseRange()->max,
52-
);
53-
54-
$upperCount = random_int(
55-
$generatorConfig->getUppercaseRange()->min,
56-
$generatorConfig->getUppercaseRange()->max,
57-
);
58-
59-
$numberCount = random_int(
60-
$generatorConfig->getNumbersRange()->min,
61-
$generatorConfig->getNumbersRange()->max,
62-
);
21+
$characterCounts = $this->calculateCharacterCounts($generatorConfig);
22+
$this->populatePassword($password, $generatorConfig, $characterCounts);
6323

64-
$symbolCount = random_int(
65-
$generatorConfig->getSymbolsRange()->min,
66-
$generatorConfig->getSymbolsRange()->max,
67-
);
68-
69-
for($i = 0; $i < $lowerCount; $i++) {
70-
$password[] = $this->pickRandom(self::POOL_CHARACTERS, $generatorConfig->getDoNotUse());
71-
}
72-
73-
for($i = 0; $i < $upperCount; $i++) {
74-
$password[] = strtoupper($this->pickRandom(self::POOL_CHARACTERS, $generatorConfig->getDoNotUse()));
75-
}
76-
77-
for($i = 0; $i < $numberCount; $i++) {
78-
$password[] = $this->pickRandom(self::POOL_NUMBERS, $generatorConfig->getDoNotUse());
79-
}
24+
shuffle($password);
25+
$password = $this->avoidConsecutiveCharacters($password);
26+
$this->ensureAlphabeticStart($password, $generatorConfig, $characterCounts);
8027

81-
for($i = 0; $i < $symbolCount; $i++) {
82-
$password[] = $this->pickRandom(self::POOL_SYMBOLS, $generatorConfig->getDoNotUse());
83-
}
28+
return implode("", $password);
29+
}
8430

85-
shuffle($password);
31+
/**
32+
* @param PasswordGeneratorConfig $config
33+
* @return array
34+
* @throws RandomException
35+
*/
36+
private function calculateCharacterCounts(PasswordGeneratorConfig $config): array {
37+
return [
38+
'lowercase' => random_int($config->lowercase->min, $config->lowercase->max),
39+
'uppercase' => random_int($config->uppercase->min, $config->uppercase->max),
40+
'numbers' => random_int($config->numbers->min, $config->numbers->max),
41+
'symbols' => random_int($config->symbols->min, $config->symbols->max),
42+
];
43+
}
8644

87-
$password = $this->avoidConsecutiveCharacters($password);
45+
/**
46+
* @param array $password
47+
* @param PasswordGeneratorConfig $config
48+
* @param array $characterCounts
49+
* @return void
50+
* @throws EmptyPoolException
51+
* @throws RandomException
52+
*/
53+
private function populatePassword(array &$password, PasswordGeneratorConfig $config, array $characterCounts): void {
54+
$this->addCharacters($password, PoolType::CHARACTERS, $characterCounts['lowercase'], $config->doNotUse);
55+
$this->addCharacters($password, PoolType::CHARACTERS, $characterCounts['uppercase'], $config->doNotUse, true);
56+
$this->addCharacters($password, PoolType::NUMBERS, $characterCounts['numbers'], $config->doNotUse);
57+
$this->addCharacters($password, PoolType::SYMBOLS, $characterCounts['symbols'], $config->doNotUse);
58+
}
8859

89-
if($generatorConfig->getAlwaysStartWithCharacter() &&
90-
($lowerCount > 0 || $upperCount > 0)) {
60+
/**
61+
* @param array $password
62+
* @param PasswordGeneratorConfig $config
63+
* @param array $characterCounts
64+
* @return void
65+
*/
66+
private function ensureAlphabeticStart(array &$password, PasswordGeneratorConfig $config, array $characterCounts): void {
67+
if($config->alwaysStartWithCharacter && ($characterCounts['lowercase'] > 0 || $characterCounts['uppercase'] > 0)) {
9168
foreach($password as $index => $char) {
9269
if(ctype_alpha($char)) {
9370
if($index !== 0) {
94-
[
95-
$password[0],
96-
$password[$index],
97-
] =
98-
[
99-
$char,
100-
$password[0],
101-
];
71+
$this->swapArrayElements($password, 0, $index);
10272
}
103-
10473
break;
10574
}
10675
}
10776
}
77+
}
10878

109-
return implode("", $password);
79+
/**
80+
* @param array $password
81+
* @param PoolType $poolType
82+
* @param int $count
83+
* @param array $doNotUse
84+
* @param bool $uppercase
85+
* @return void
86+
* @throws EmptyPoolException
87+
* @throws RandomException
88+
*/
89+
private function addCharacters(array &$password, PoolType $poolType, int $count, array $doNotUse, bool $uppercase = false): void {
90+
for($i = 0; $i < $count; $i++) {
91+
$char = $this->pickRandom($poolType, $doNotUse);
92+
$password[] = $uppercase ? strtoupper($char) : $char;
93+
}
11094
}
11195

11296
/**
113-
* @param string $poolMode
97+
* @param PoolType $poolType
11498
* @param array $doNotUse
11599
* @return string
116100
* @throws EmptyPoolException
117101
* @throws RandomException
118102
*/
119-
private function pickRandom(string $poolMode, array $doNotUse): string {
120-
$pool = match ($poolMode) {
121-
self::POOL_NUMBERS => self::NUMBERS,
122-
self::POOL_SYMBOLS => self::SYMBOLS,
123-
default => self::CHARACTERS,
103+
private function pickRandom(PoolType $poolType, array $doNotUse): string {
104+
$pool = match ($poolType) {
105+
PoolType::NUMBERS => CharacterPool::NUMBERS->getPool(),
106+
PoolType::SYMBOLS => CharacterPool::SYMBOLS->getPool(),
107+
default => CharacterPool::CHARACTERS->getPool(),
124108
};
125109

126110
$pool = str_split($pool);
127111
$pool = array_filter($pool, fn($char) => !in_array(strtolower($char), array_map("strtolower", $doNotUse)));
128112
$pool = array_values($pool);
129113

130114
if(!count($pool)) {
131-
throw new EmptyPoolException("The pool '$poolMode' is empty");
115+
throw new EmptyPoolException("The pool '{$poolType->value}' is empty");
132116
}
133117

134118
return $pool[random_int(0, count($pool) - 1)];
135119
}
136120

121+
/**
122+
* @param array $array
123+
* @param int $index1
124+
* @param int $index2
125+
* @return void
126+
*/
127+
private function swapArrayElements(array &$array, int $index1, int $index2): void {
128+
[$array[$index1], $array[$index2]] = [$array[$index2], $array[$index1]];
129+
}
130+
137131
/**
138132
* @param array $password
139133
* @return array
140134
*/
141135
private function avoidConsecutiveCharacters(array $password): array {
142136
$attempts = 0;
143137

144-
while($attempts < 5) {
138+
while($attempts < self::MAX_CONSECUTIVE_AVOIDANCE_ATTEMPTS) {
145139
$hasConsecutive = FALSE;
146140

147141
for($i = 1; $i < count($password); $i++) {
@@ -154,14 +148,7 @@ private function avoidConsecutiveCharacters(array $password): array {
154148
$password[$j] !== $password[$i] &&
155149
$password[$j] !== $password[$i - 1]
156150
) {
157-
[
158-
$password[$i],
159-
$password[$j],
160-
] =
161-
[
162-
$password[$j],
163-
$password[$i],
164-
];
151+
$this->swapArrayElements($password, $i, $j);
165152
break;
166153
}
167154
}

‎src/PasswordGeneratorConfig.php‎

Lines changed: 6 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -14,55 +14,14 @@
1414
* @param array $doNotUse
1515
*/
1616
public function __construct(
17-
private PoolRange $lowercase = new PoolRange(4, 4),
18-
private PoolRange $uppercase = new PoolRange(2, 2),
19-
private PoolRange $numbers = new PoolRange(1, 1),
20-
private PoolRange $symbols = new PoolRange(1, 1),
21-
private bool $alwaysStartWithCharacter = FALSE,
22-
private array $doNotUse = [],
17+
public PoolRange $lowercase = new PoolRange(4, 4),
18+
public PoolRange $uppercase = new PoolRange(2, 2),
19+
public PoolRange $numbers = new PoolRange(1, 1),
20+
public PoolRange $symbols = new PoolRange(1, 1),
21+
public bool $alwaysStartWithCharacter = FALSE,
22+
public array $doNotUse = [],
2323
) {}
2424

25-
/**
26-
* @return PoolRange
27-
*/
28-
public function getLowercaseRange(): PoolRange {
29-
return $this->lowercase;
30-
}
31-
32-
/**
33-
* @return PoolRange
34-
*/
35-
public function getUppercaseRange(): PoolRange {
36-
return $this->uppercase;
37-
}
38-
39-
/**
40-
* @return PoolRange
41-
*/
42-
public function getNumbersRange(): PoolRange {
43-
return $this->numbers;
44-
}
45-
46-
/**
47-
* @return PoolRange
48-
*/
49-
public function getSymbolsRange(): PoolRange {
50-
return $this->symbols;
51-
}
52-
53-
/**
54-
* @return bool
55-
*/
56-
public function getAlwaysStartWithCharacter(): bool {
57-
return $this->alwaysStartWithCharacter;
58-
}
59-
60-
/**
61-
* @return array
62-
*/
63-
public function getDoNotUse(): array {
64-
return $this->doNotUse;
65-
}
6625

6726
/**
6827
* @return int

‎src/PoolType.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace Kristos80\PasswordGenerator;
5+
6+
enum PoolType: string {
7+
case CHARACTERS = "characters";
8+
case NUMBERS = "numbers";
9+
case SYMBOLS = "symbols";
10+
}

‎tests/PasswordGeneratorTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ private function callAvoidConsecutiveCharacters(array $input): array {
4747
$generator = new PasswordGenerator();
4848
$method = new ReflectionMethod(PasswordGenerator::class, "avoidConsecutiveCharacters");
4949

50-
return $method->invoke($generator, $input, 5);
50+
return $method->invoke($generator, $input);
5151
}
5252

5353
/**
@@ -81,7 +81,7 @@ public function testUnfixablePassword(): void {
8181
"a",
8282
"a",
8383
];
84-
$output = $this->callAvoidConsecutiveCharacters($input, 2);
84+
$output = $this->callAvoidConsecutiveCharacters($input);
8585

8686
$consecutiveExists = FALSE;
8787
for($i = 1; $i < count($output); $i++) {

0 commit comments

Comments
 (0)