Make extensible #8
Conversation
…e, but ensure default behavior is kept.
| ->andReturn('mockMode'); | ||
|
|
||
| $contentGetter = $modes->controlMode(null, $mockRestRequest); | ||
| $this->assertTrue(is_a($contentGetter, CreatePostsImplementation::class)); |
There was a problem hiding this comment.
Change this assert to use assertInstanceOf instead of the assertTrue(is_a())
$this->assertInstanceOf(CreatePostsImplementation::class, $contentGetter);
Tests/Unit/ModeControlTest.php
Outdated
| { | ||
| $modes = new Modes(); | ||
| $this->assertTrue(is_a($modes->factory(''), PostsGenerator::class)); | ||
| $this->assertTrue(is_a($modes->factory(PostsGenerator::class), PostsGenerator::class)); |
There was a problem hiding this comment.
Change these asserts like the one above to use assertInstanceOf:
$this->assertInstanceOf(PostsGenerator::class, $modes->factory(''));
$this->assertInstanceOf($modes->factory(PostsGenerator::class), $modes->factory(''));
Tests/Unit/ModeControlTest.php
Outdated
| ->once() | ||
| ->andReturn('default'); | ||
|
|
||
| $this->assertTrue(is_a($modes->controlMode(null, $mockRestRequest), PostsGenerator::class)); |
There was a problem hiding this comment.
Change this assert to use assertInstanceOf:
$this->assertInstanceOf(PostsGenerator::class, $modes->controlMode(null, $mockRestRequest));
Tests/Unit/ModeControlTest.php
Outdated
| ->andReturn('default'); | ||
|
|
||
| $modes->controlMode(null, $mockRestRequest); | ||
| $this->assertTrue(Filters\applied(Modes::FILTER_NAME) > 0); |
There was a problem hiding this comment.
Change the assert to assertGreaterThan:
$this->assertGreaterThan(0, Filters\applied(Modes::FILTER_NAME));
src/Modes.php
Outdated
| if (is_a($contentGetter, ContentGetterContract::class)) { | ||
| return $contentGetter; | ||
| } | ||
| switch ($mode) { |
There was a problem hiding this comment.
Is the point to this switch to be a placeholder for future extension or even discussion? In its current design, we could just return $this->factory(''); and get rid of the switch code structure.
src/Modes.php
Outdated
| switch ($mode) { | ||
| default: | ||
| return $this->factory(''); | ||
| break; |
There was a problem hiding this comment.
This isn't necessary as we are returning and the break will never be reached.
src/Modes.php
Outdated
| */ | ||
| public function factory(string $type) : ContentGetterContract | ||
| { | ||
| switch ($type) { |
There was a problem hiding this comment.
Same as above, what is the point of this switch? In its current design, it will always create a new instance of the PostsGenerator and then return it. Why not just do line 57 and remove the rest of this code?
In addition to #6 and #7
Discussion here: #6 (comment)