0
\$\begingroup\$

Is it possible to simplify getRows() or it is fine as it is now?

protected $container;
public function __construct(ContainerInterface $container) {
    $this->container = $container;
    $this->em = $this->getDoctrine()->getManager();
}

/*
* @param    $ntt              name of the entity to fetch
* @param    $cols             columns to fetch
* @param    $where            array of data to be matched (for example array('id'=>1))
* @param    $result_type      type of result required. (object|array)
*/
public function getRows($ntt,$cols=array(),$where=array(),$result_type=null){
    $data = 'nulllllll';
    if($ntt && $where && is_array($where)){
        $i = 1;
        $filter = '';
        foreach ($where as $key => $value) {
            if(is_array($value)){
                $filter = $filter.($filter ? ' AND ' : '')."tm.$key in (:v$i)";
            }else{
                $filter = $filter.($filter ? ' AND ' : '')."tm.$key = :v$i";
            }
            $param['v'.$i] = $value;
            $i++;
        }

        $select = 'tm';
        if($cols){
            $select = '';
            foreach ($cols as $col) {
                $select = $select.($select ? ', ' : '')."tm.$col";
            }
        }

        $ntt = 'AppBundle:'.$ntt;
        $data = $this->em->getRepository($ntt)->createQueryBuilder('tm')
        ->select($select)
        ->where($filter)
        ->setParameters($param)
        ->getQuery();
        if($result_type == 'array'){
            $data = $data->getArrayResult();
        }else{
            $data = $data->getResult();
        }
    }
    return $data;
}

Calling:

getRows('My_user',array('lName','llname'),array('id'=>array(1,4,50)),'array')

Where My_user is my entity name and lName and llname are the columns I want to fetch.

Will output:

Array
(
    [0] => Array
        (
            [lName] => Abc
            [llname] => Sha
        )

    [1] => Array
        (
            [lName] => And
            [llname] => Bainbri
        )

    [2] => Array
        (
            [lName] => Ken
            [llname] => Lee
        )

)
\$\endgroup\$
1
  • \$\begingroup\$ You should change the variable name to $query because that is what getQuery() returns. Using $data = $data->someMethod() isn't a good practice for obvious reasons. \$\endgroup\$ Jul 5, 2018 at 21:09

2 Answers 2

1
\$\begingroup\$

I'd suggest your code to be rewritten like this (please, take note of the inline comments):

<?php

// Requires PHP >= 5.4

public function getRows($ntt, $cols = array(), $where = array(), $result_type = null)
{
    $data = 'nulllllll';

    // Early return after checking arguments,
    // instead of nesting the rest of the code inside the if{}
    if (!($ntt && $where && is_array($where))) {
        return $data;
    }

    $filters = [];
    $param = [];

    // The callback function here uses outer variables: `$filters` and `$param`,
    // that are passed by reference `&`
    array_walk($where, function($value, $key) use (&$filters, &$param) {
        static $i = 0;
        $i++;

        $placeholder = "v{$i}";
        $condition = is_array($value) ? "in (:{$placeholder})" : "= :{$placeholder}";
        $filters[] = "tm.{$key} {$condition}";
        $param[$placeholder] = $value;
    });

    // Use `implode` to concatenate the `$filters` into a string
    $filter = implode(' AND ', $filters);

    $select = 'tm';
    if (!empty($cols) && is_array($cols)) {
        $select = implode(', ', array_map(function($column) {
                return "tm.{$column}";
            }, $cols));
    }

    $repository = "AppBundle:{$ntt}";
    $data = $this->em->getRepository($repository)->createQueryBuilder('tm')
        ->select($select)
        ->where($filter)
        ->setParameters($param)
        ->getQuery();

    if ($result_type === 'array') {
        return $data->getArrayResult();
    }

    return $data->getResult();
}
\$\endgroup\$
2
  • \$\begingroup\$ That's nice, but the line $select = 'tm.*'; will produce error if $cols is empty. It should be $select = 'tm';. \$\endgroup\$
    – DEarTh
    Mar 19, 2018 at 5:52
  • \$\begingroup\$ @JitendraSoftgrid, yes, my mistake, sorry. Fixed that. Thanks. \$\endgroup\$
    – Joe Black
    Mar 19, 2018 at 10:26
1
\$\begingroup\$

This function of yours defeats the very idea behind ORM. Your own solution is less flexible and less secure than original Doctrine. Let alone all the type hinting provided by your IDE will be unavailable.

Instead of calling this function you must be using

$this->em->getRepository(My_user::class)->find([1,4,50])->getArrayResult();

In case you need to apply another filters, then just use findBy() instead of find().

You must understand that the current usage pattern for Doctrine has a strong reason behind it. By implementing your function you will introduce a lot of potential problems, making them hard to debug. Making your code less flexible at the same time. Let alone a tight coupling, making your app to rely on such considerations like the bundle is always called AppBundle, the entity manager is always the same etc.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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