1
\$\begingroup\$

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

\$\endgroup\$

1 Answer 1

0
\$\begingroup\$
$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...

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.