Skip to content

Commit b1ac552

Browse files
committed
improved maintainability
1 parent 4ee9d1b commit b1ac552

File tree

8 files changed

+57
-20
lines changed

8 files changed

+57
-20
lines changed

src/CommandFactory.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ class CommandFactory implements CommandFactoryInterface
66
{
77
private $parser;
88
private $fileReader;
9-
private $displayMode;
109

1110
public function __construct($parser, $fileReader)
1211
{
@@ -28,7 +27,7 @@ public function getCommand(string $commandType): ?CommandInterface
2827
$requestedCommand = new DisplayCommand();
2928
break;
3029
default:
31-
return null;
30+
throw new DifferException("internal error: unknown command factory option\n");
3231
}
3332
return $requestedCommand;
3433
}

src/DifferException.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
namespace Differ;
4+
5+
class DifferException extends \Exception
6+
{
7+
public function __construct(string $message)
8+
{
9+
parent::__construct($message);
10+
}
11+
}

src/DisplayCommand.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
class DisplayCommand implements CommandInterface
66
{
7-
// Property to store displaying mode
7+
// Property to store displaying mode for debug
88
private string $mode;
99
public const AVAILABLE_MODES = [
1010
"differents",
@@ -27,7 +27,7 @@ public function execute(CommandInterface $command = null): CommandInterface
2727
print_r($command->getFilesContent());
2828
break;
2929
default:
30-
throw new \Exception("internal error: unknown mode for display\n");
30+
throw new DifferException("internal error: unknown mode for display\n");
3131
}
3232
}
3333

src/FileReader.php

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,28 +26,24 @@ public function readFile(string $filename, bool $isArray = null): ?array
2626
$fileFormat = end($fileNameParts);
2727
if ($fileFormat === 'json') {
2828
$handle = fopen($filename, "r");
29-
$result = json_decode(fread($handle, self::MAX_FILE_SIZE), $isArray);
29+
$jsonVariables = json_decode(fread($handle, self::MAX_FILE_SIZE), $isArray);
3030
fclose($handle);
31-
$type = gettype($result);
31+
$type = gettype($jsonVariables);
3232
if ($type === 'object') {
33-
return get_object_vars($result);
33+
$fileContentArray = get_object_vars($jsonVariables);
3434
} elseif ($type === 'array') {
35-
return $result;
35+
$fileContentArray = $jsonVariables;
3636
}
3737
} elseif ($fileFormat === 'yaml' || $fileFormat === 'yml') {
3838
$handle = fopen($filename, "r");
39-
$result = Yaml::parse(fread($handle, self::MAX_FILE_SIZE), Yaml::PARSE_OBJECT_FOR_MAP);
39+
$yamlVariables = Yaml::parse(fread($handle, self::MAX_FILE_SIZE), Yaml::PARSE_OBJECT_FOR_MAP);
4040
fclose($handle);
41-
$type = gettype($result);
42-
if ($type === 'object') {
43-
return get_object_vars($result);
44-
} elseif ($type === 'array') {
45-
return $result;
46-
}
41+
$fileContentArray = get_object_vars($yamlVariables);
4742
} else {
48-
throw new \Exception("Unknown files format: \n" .
49-
"use .json, .yaml (.yml) enstead \n");
43+
throw new DifferException("unknown files format: use json, yaml (yml) enstead\n");
5044
}
45+
46+
return $fileContentArray;
5147
} else {
5248
return null;
5349
}

src/Gendiff.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ function runGendiff(
1111
$consoleApp = new ConsoleApp($commandFactory);
1212
try {
1313
$consoleApp->run();
14-
} catch (\Exception $e) {
14+
} catch (DifferException $e) {
1515
print_r("{$e->getMessage()}");
1616
}
1717
}

tests/CommandFactoryTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@
1111
use Differ\DisplayCommand;
1212
use Differ\FileReader;
1313
use Differ\DocoptDouble;
14+
use Differ\DifferException;
1415

1516
#[CoversClass(CommandFactory::class)]
1617
#[CoversClass(CommandLineParser::class)]
1718
#[CoversClass(FilesDiffCommand::class)]
1819
#[CoversClass(DisplayCommand::class)]
1920
#[CoversClass(DocoptDouble::class)]
2021
#[CoversClass(FileReader::class)]
22+
#[CoversClass(DifferException::class)]
2123
class CommandFactoryTest extends TestCase
2224
{
2325
private $commandFactory;
@@ -45,6 +47,9 @@ public function testGetCommand()
4547
$this->assertInstanceOf(DisplayCommand::class, $this->commandFactory->getCommand('show'));
4648

4749
// Test for undefined command
48-
$this->assertNull($this->commandFactory->getCommand('undefined'));
50+
$this->expectException(DifferException::class);
51+
$this->expectExceptionMessageMatches("/internal error: unknown command factory option\\n/");
52+
53+
$this->commandFactory->getCommand('undefined');
4954
}
5055
}

tests/DisplayCommandTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
use PHPUnit\Framework\Attributes\CoversClass;
77
use PHPUnit\Framework\Attributes\CoversMethod;
88
use Differ\DisplayCommand;
9+
use Differ\DifferException;
910

1011
#[CoversClass(DisplayCommand::class)]
1112
#[CoversMethod(DisplayCommand::class, 'execute')]
13+
#[CoversClass(DifferException::class)]
1214
class DisplayCommandTest extends TestCase
1315
{
1416
private $filesContent;
@@ -74,7 +76,7 @@ public function testUnknownDisplayMode()
7476
{
7577
$displayCmd = new DisplayCommand();
7678

77-
$this->expectException(\Exception::class);
79+
$this->expectException(DifferException::class);
7880
$this->expectExceptionMessageMatches("/internal error: unknown mode for display\\n/");
7981

8082
$displayCmd->setMode("extra")->execute($this->filesDiffCmd);

tests/FilesDiffCommandTest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
use PHPUnit\Framework\Attributes\CoversClass;
77
use PHPUnit\Framework\Attributes\CoversMethod;
88
use Differ\FileReader;
9+
use Differ\DifferException;
910

1011
#[CoversClass(FilesDiffCommand::class)]
1112
#[CoversClass(FileReader::class)]
1213
#[CoversMethod(FilesDiffCommand::class, 'setFileReader')]
1314
#[CoversMethod(FilesDiffCommand::class, 'execute')]
15+
#[CoversClass(DifferException::class)]
1416
class FilesDiffCommandTest extends TestCase
1517
{
1618
private $fileNames;
@@ -25,6 +27,10 @@ protected function setUp(): void
2527
"FILE1" => __DIR__ . "/../file1.yaml",
2628
"FILE2" => __DIR__ . "/../file2.yaml"
2729
];
30+
$this->fileNames['Exception'] = [
31+
"FILE1" => __DIR__ . "/../fixtures/file1.txt",
32+
"FILE2" => __DIR__ . "/../file2.yaml"
33+
];
2834
}
2935

3036
public function testSetFileReader()
@@ -95,4 +101,22 @@ public function testExecuteForYAML()
95101
$resultDiffs
96102
);
97103
}
104+
105+
public function testExecuteForException()
106+
{
107+
$cmdLineParser = $this->createConfiguredStub(
108+
CommandLineParser::class,
109+
[
110+
'getFileNames' => $this->fileNames['Exception']
111+
]
112+
);
113+
114+
$diffCommand = new FilesDiffCommand();
115+
116+
$this->expectException(DifferException::class);
117+
$this->expectExceptionMessageMatches("/unknown files format: use json, yaml \(yml\) enstead\\n/");
118+
119+
$diffCommand->setFileReader(new FileReader())
120+
->execute($cmdLineParser);
121+
}
98122
}

0 commit comments

Comments
 (0)