Refactor namespace #8
Conversation
LauLaman
force-pushed
the
KAF_REFACTOR_NAMESPACE
branch
7 times, most recently
from
72f7ecf
to
677d39c
Compare
|
|
||
| abstract class AbstractBuilder | ||
| abstract class AbstractFactory |
There was a problem hiding this comment.
the Factory name becomes a better fit since we were not working the Builder pattern.
| } | ||
|
|
||
| public function fromData(array $data): ProfileResponse | ||
| public function fromProfileData(array $data): KvkPaginator |
There was a problem hiding this comment.
maybe use KvkPaginatorInterface as return type instead.
|
|
||
| interface KvkPaginatorFactoryInterface | ||
| { | ||
| public function fromProfileData(array $data): KvkPaginator; |
src/Client/KvkPaginator.php
Outdated
|
|
||
| final class ProfileResponse implements ResponseInterface | ||
| final class KvkPaginator implements KvkPaginatorInterface |
src/KvkClient.php
Outdated
| * @param $json | ||
| * @return mixed | ||
| */ | ||
| private function decodeJsonToArray($json) |
There was a problem hiding this comment.
type hint string as parameter type and then we can get rid of this docblock. the return type should be an array.
| * @dataProvider getNonExistingKvkNumbers | ||
| * @expectedException \Werkspot\KvkApi\Http\Adapter\Guzzle\Exception\NotFoundException | ||
| */ | ||
| public function getProfile_shouldThrowException(int $kvkNumber): void |
There was a problem hiding this comment.
maybe shouldThrowExceptionWhenItemsAreNotFound?
There was a problem hiding this comment.
I this case I think having a more explicit way is better since we are moving away from the happy flow. I think the WhenItemsAreNotFound is not necessary since the data provider already tells us that these items are nonExisting. so we should expect an exception.
There was a problem hiding this comment.
I see what you mean. I usually would not rely on the dataProvider to be explicit, the first place I look is at the method name.
| { | ||
| /** | ||
| * @test | ||
| * @dataProvider getArrayData | ||
| */ | ||
| public function fromArray(array $data): void | ||
| { | ||
| $builder = new BusinessActivityBuilder(); | ||
| $Factory = new BusinessActivityFactory(); |
There was a problem hiding this comment.
start variable name with lowercase letter.
| { | ||
| /** | ||
| * @test | ||
| * @dataProvider getAddressData | ||
| */ | ||
| public function fromArray(array $data): void | ||
| { | ||
| $builder = new AddressBuilder(); | ||
| $Factory = new AddressFactory(); |
There was a problem hiding this comment.
start variable name with lowercase letter.
| { | ||
| /** | ||
| * @test | ||
| * @dataProvider getArrayData | ||
| */ | ||
| public function fromArray(array $data): void | ||
| { | ||
| $builder = new TradeNamesBuilder(); | ||
| $output = $builder->fromArray($data); | ||
| $Factory = new TradeNamesFactory(); |
|
|
||
| $builder = new CompanyBuilder($tradeNamesBuilder, $businessActivityBuilder, $addressBuilder); | ||
| $Factory = new CompanyFactory($tradeNamesFactory, $businessActivityFactory, $addressFactory); |
LauLaman
force-pushed
the
KAF_REFACTOR_NAMESPACE
branch
from
677d39c
to
ab38bab
Compare
Get Rid of the authentication code: implement in guzzle config
Refactor namespace