1. Preferrably PHP 7.4 instead of PHP 7.3
2. Maximize mo yung pagiging PHP 7.4 like typed properties
Instead of:
```php
...
protected $id;
```
Gawin mong
```php
...
protected string $id;
```
3. Fix mo yung mga nullable properties
Instead na
``` php
protected ?Factory $factory;
public function __construct(Factory $factory = null)
```
gawin mong:
```php
protected ?Factory $factory;
public function __construct(?Factory $factory = null)
```
4. sa may `CustomerMiddleware::findCustomer`, do [happy path](https://en.wikipedia.org/wiki/Happy_path).
5. Instead of using the `EntityManager`:
```php
// Foo Service
private EntityManagerInterface $entityManger;
public function __construct(EntityManagerInterface $entityManager)
{
$this->entityManager = $entityManager;
}
public function findCustomer(int $customerId): Customer
{
$customerRepository = $this->entityManager->getRepository(Customer::class);
$customer = $customerRepository->find($customerId);
}
```
Inject the `CustomerRepository` directly. See: http://www.laraveldoctrine.org/docs/1.4/orm/repositories
```php
// FooService
private CustomerRepositoryInterface $customerRepository;
public function __construct(CustomerRepositoryInterface $customerRepository)
{
$this->customerRepository = $customerRepository;
}
public function findCustomer(int $customerId): Customer
{
return $this->customerRepository->find($customerId);
}
```
6. Sa `CustomerImportModel` line 33
```php
...
if ($customer !== null) {
$customer->setEmail(Arr::get($row, 'email'));
}
...
```
I'm not sure kung magiging `null` parin siya dyan since ni seset mo line 23
```php
...
$customer = ($customer ?? new Customer())
```
7. Sa router, alam ko instead of grouping with the namespace, pwede mong directly yung class na parang, instead of:
```php
$router->get('customers', [
'uses' => 'CustomerController@show'
...
]);
```
Pwede atang ganito na:
```php
$router->get('customers', [
'uses' => [CustomerController::class, 'show'],
...
]);
```
8. Yung `ImporterManager` mo itest mo mabuti, baka magka side effect ka sa line 41
```
...
40 return new JsonDriver(
41 $this->config->get('customer')[$this->getDefaultDriver()],
42 $this->factory->baseUrl($this->config->get('customer.json.url'))->asForm()
43 );
```
kapag nagbago yung default driver.
9. Parang wala masyadong flexibility yung `JsonDriver::generateQueryParams`, kasi you have `$options` argument pero `$options['count']` lang naman pala yung mababago, the rest hindi.
10. Yung `CustomerImportModel`, masyadong tali ata sa `randomuser.me`, if pwede mong maidefine din sa driver much better, medyo tricky.
11. Yung `tests` mo gawin mong PSR-4 compliance, yung with Namespace
12. Yung mga test cases mo, be consistent sa pag gamit ng methods, instead of:
```php
...
public function test_if_can_paginate(): void
```
Gawin mong:
```php=
...
public function testIfCanPaginate(): void
```
13. Yung test mo sa `CustomerTest::test_if_can_query_results`, parang walang sense sya, yeah 200 status pero hindi na veverify kung tama ba yung order talaga. Add ka din ng mga query params na mali like `?order=FOO`
```php=
...
$this->get('customers/?order=ASC');
...
$data = Collection::make($this->response->json('data'));
$ids = $data->pluck('id');
self::assertSame([1, 2, 3, 4, 5, 6, 7, 8, 9, 10], $ids->toArray());
$this->get('customers/?order=DESC');
$this->assertResponseOk();
$data = Collection::make($this->response->json('data'));
$ids = $data->pluck('id');
self::assertSame([10, 9, 8, 7, 6, 5, 4, 3, 2, 1], $ids->toArray());
```
15. Same goes sa `CustomerTest::test_if_can_paginate`, how are sure na page 2 nga talaga yung lumabas
16. Add ka `@mixin` PHPDocs sa mga JsonResource
17. Parang maling class yung nainject mo sa `CustomerImportTest` line 63
---
18. `CustomerImporterTest::testIfCanCreateCustomerFromEntity` kahit wala na ito.
19. Pwede mo din maximize pa yung PHP 7.4 na shorthand function
Instead of:
```php=
$this->callable(function () {
return 'hey';
});
```
Pwedeng:
```php=
$this->callable(fn() => 'hey');
```
20. Remove mo na yung pagiging nullable ng `Dispatcher` na dependency sa `CustomerImporter`, baka magkaside effect ka kapag walang napasa na `Dispatcher`, if that's the case never matatawag yung `CustomerImportEvent` hence yung pagadvance ng bar hindi narin magagawa