I want to improve quality of this symfony rest endpoint (solid principle, Kiss, best practice...) Can you review my code please?
Symfony controllController function that return json list of product
/**
* @Route("/api/products/my_list/{number}", methods={"GET"})
* @param Security $security
* @param BeamyAPI $beamyAPI
* @param Request $request
* @param ProductService $productService
* @return string
*/
public function myList(
Security $security,
BeamyAPI $beamyAPI,
Request $request,
ProductService $productService,
int $number = 20
)
{
$productListUser = $this->em->getRepository(ProductAdmin::class)
->findProductsUser(
$security->getUser(),
$number
);
return new JsonResponse(
$productListUser,
Response::HTTP_OK
);
}
Trait to format array response
Trait ArrayFormat
{
/**
* format array for user product endpoint
*
* @param array $data
* @return array
*/
public function formatUserProduct($product) : array
{
return [
'id' => $product['id'],
'name' => $product['name'],
'notifications' => '',
'logo' => [
'contentUrl' => $product['logo']['contentUrl']
]
];
}
}
Repository permit to get array of products list for one user
/**
* get product list of user
*
* @param UserInterface $user
* @param integer $number
* @return array|null
*/
public function findProductsUser(UserInterface $user, int $number) :?array
{
$listProductUser = $this->em->getRepository(ProductAdmin::class)->findBy(
['user' => $user],
['product' => 'ASC'],
$number
);
$res = [];
array_walk($listProductUser, function(&$productUser){
$product = $this->productService->getProductInfo($productUser->getProduct());
$productUser = ArrayFormat::formatUserProduct($product);
});
return $listProductUser;
}
Thanks
1 Answer 1
$productListUser = $this->em->getRepository(ProductAdmin::class)
->findProductsUser(...
Than
public function findProductsUser(UserInterface $user, int $number) :?array
{
$listProductUser = $this->em->getRepository(ProductAdmin::class)->findBy(...
seems to me that findProductsUser
is method of whatever is returned by $em->getRepository(ProductAdmin::class)
why is that method retrieving itself from entity manager? Shouldn't it be just $this->findBy(...
?
Further, the responsibility of formatting the entity should not be done by the repository. You need to either wrap the repository with the formatter into another class, or do it in the controller. Also there is no reason why the formatter must be a trait. Make it a service and inject it to the entire controller through contructor if all methods of the controller need it, or have it injected in an action method, like you do with Security
, BeamAPI
, etc...