Conversation
Improve travis build
|
Parameter naming strategy for blueprint builder -> todo |
| $value = $this->prepareData($value); | ||
| } | ||
|
|
||
| if (!is_int($key)) { |
There was a problem hiding this comment.
Key can any, not only int or string
There was a problem hiding this comment.
The key can either be an integer or a string. The value can be of any type.
Other types are casted underlying to string or int.
| { | ||
| $pattern = $this->generator->create($class); | ||
|
|
||
| return eval($pattern); |
There was a problem hiding this comment.
You should check this value before return to avoid typeerror
| private $nullable; | ||
|
|
||
| /** @param mixed $defaultValue */ | ||
| public function __construct(string $type, string $name, bool $nullable = false, $defaultValue = null) |
There was a problem hiding this comment.
IMO it's better to expects all parameters
| return $this->name; | ||
| } | ||
|
|
||
| public function withDefaultValue(): bool |
There was a problem hiding this comment.
Looks like method which changes immutable object. hasDefaultValue?
|
|
||
| public function withDefaultValue(): bool | ||
| { | ||
| return null !== $this->defaultValue; |
| return $this->serializeObjectListNode($node); | ||
| } | ||
|
|
||
| throw new BuildingError(); |
There was a problem hiding this comment.
Missing details about this error.
| { | ||
| try { | ||
| return $this->store[$class]; | ||
| } catch (Throwable $exception) { |
src/Builder/Reflection.php
Outdated
| if (null !== $constructorMethod) { | ||
| $parameters = iterator_to_array($this->collect($constructorMethod, $data)); | ||
| /** @var \Traversable $iterator */ | ||
| $iterator = $this->collect($constructorMethod, $data); |
| 'int', | ||
| 'float', | ||
| 'double', | ||
| 'mixed', |
There was a problem hiding this comment.
In context of this private function, it is. But I just didn't find better name so I'm open for alternatives name.
| throw new BuildingError('Can not resolve namespace for class ' . $className); | ||
| } | ||
|
|
||
| private function endsWith(string $haystack, string $needle): bool |
There was a problem hiding this comment.
Maybe move it to function or trait
There was a problem hiding this comment.
Not now, because I will refactor Builder\Reflection to use PhpDocParser\PhpStan
| { | ||
| public static function setUpBeforeClass(): void | ||
| { | ||
| static::$builder = new Reflection(new SnakeCase()); |
There was a problem hiding this comment.
Because Builders are stateless and no needs to recreate in every test case.
| use PHPUnit\Framework\TestCase; | ||
| use RstGroup\ObjectBuilder\Builder\Blueprint\Factory\CodeGenerator\PatternStore\Filesystem; | ||
|
|
||
| class FilesystemTest extends TestCase |
There was a problem hiding this comment.
There are libs to mock file system
There was a problem hiding this comment.
Yes, but is it good cause to use his? In integration test I don't want mock object of my integration - filesystem.
2 participants
No description provided.