Múltiples argumentos en llamada de función frente a matriz única

24

Tengo una función que toma un conjunto de parámetros y luego se aplica a ellos como condiciones para una consulta SQL. Sin embargo, si bien preferí una matriz de argumento único que contenga las condiciones mismas:

function searchQuery($params = array()) {
    foreach($params as $param => $value) {
        switch ($param) {
            case 'name':
                $query->where('name', $value);
                break;
            case 'phone':
                $query->join('phone');
                $query->where('phone', $value);
                break;
        }
    }
}

Mi colega prefirió enumerar todos los argumentos explícitamente en su lugar:

function searchQuery($name = '', $phone = '') {
    if ($name) {
        $query->where('name', $value);
    }

    if ($phone) {
        $query->join('phone');
        $query->where('phone', $value);
    }
}

Su argumento fue que al enumerar los argumentos explícitamente, el comportamiento de la función se vuelve más evidente, en lugar de tener que profundizar en el código para descubrir cuál era el argumento misterioso $param.

Mi problema fue que esto se vuelve muy detallado cuando se trata con muchos argumentos, como 10+. ¿Hay alguna práctica preferida? Mi peor de los casos sería ver algo como lo siguiente:

searchQuery('', '', '', '', '', '', '', '', '', '', '', '', 'search_query')

xiankai
fuente
1
Si la función espera claves específicas como parámetros, al menos esas claves deben documentarse en un DocBlock, de esa manera los IDE pueden mostrar la información relevante sin tener que profundizar en el código. en.wikipedia.org/wiki/PHPDoc
Ilari Kajaste
2
Consejo de rendimiento: foreaches innecesario en este caso, podría usarlo en if(!empty($params['name']))lugar de foreachy switch.
chiborg
1
Ahora tiene un método que utiliza. Sugeriría echar un vistazo aquí: book.cakephp.org/2.0/en/models/… para crear más métodos. Incluso pueden generarse mágicamente para hallazgos estándar y desarrollarse en forma personalizada para búsquedas específicas. En general, eso hace una API clara para los usuarios del modelo.
Luc Franken
1
relacionado: Pasar objeto compuesto para parámetros
mosquito
2
Una nota sobre el 'consejo de rendimiento' anterior: no lo use ciegamente !empty($params['name'])para probar parámetros; por ejemplo, la cadena "0" estaría vacía. Es mejor usar array_key_existspara verificar la clave, o issetsi no te importa null.
AmadeusDrZaius

Respuestas:

27

En mi humilde opinión su colega es correcto para el ejemplo anterior. Su preferencia puede ser breve, pero también es menos legible y, por lo tanto, menos mantenible. Haga la pregunta por qué molestarse en escribir la función en primer lugar, ¿qué 'trae a la mesa' su función? Tengo que entender lo que hace y cómo lo hace, en gran detalle, solo para usarla. Con su ejemplo, aunque no soy un programador de PHP, puedo ver suficientes detalles en la declaración de funciones que no tengo que preocuparme por su implementación.

En cuanto a una mayor cantidad de argumentos, eso normalmente se considera un olor a código. Por lo general, la función está tratando de hacer demasiado? Si encuentra una necesidad real de una gran cantidad de argumentos, es probable que estén relacionados de alguna manera y pertenezcan juntos en una o unas pocas estructuras o clases (tal vez incluso una matriz de elementos relacionados, como líneas en una dirección). Sin embargo, pasar una matriz no estructurada no hace nada para solucionar los olores del código.

Mattnz
fuente
En cuanto a la necesidad de una gran cantidad de argumentos, la función esencialmente toma cero o más argumentos, y luego limita el resultado establecido por esos argumentos. Los argumentos en sí mismos no tienen mucho que ver entre sí (como cláusulas SQL distintas), y puede que ni siquiera tengan la misma estructura (uno puede ser un DÓNDE simple, pero otro requeriría varias UNIONES además del DÓNDE). ¿Todavía se consideraría un olor a código en este caso específico?
xiankai
2
@xiankai En ese ejemplo, tal vez haría un parámetro de matriz para los whereargumentos, uno para los joinespecificadores, etc.
Jan Doggen
¿Qué sucede si uso setter / getter en su lugar y no paso ningún argumento? ¿Es una mala práctica? ¿No es un propósito usar setter / getter?
lyhong
Desafiaría que la preferencia del OP sea "menos legible" (¿cómo?) Y menos mantenible searchQuery ('', '', '', '', 'foo', '', '', '', 'bar') es mucho menos legible o mantenible que searchQuery (['q' => 'foo', 'x' => 'bar']) Una gran cantidad de argumentos tampoco es necesariamente un olor a código; una consulta (), por ejemplo. E incluso para un número menor de argumentos, la falta de coherencia en el orden de los argumentos que ocurre cuando los argumentos se pasan directamente ilustra la mala idea de codificar los parámetros. Solo mire las funciones de cadena y matriz en PHP para dicha inconsistencia.
MikeSchinkel
4

Mi respuesta es más o menos independiente del idioma.

Si el único propósito de agrupar argumentos en una estructura de datos compleja (tabla, registro, diccionario, objeto ...) es pasarlos como un todo a una función, mejor evítelo. Esto agrega una capa inútil de complejidad y hace que su intención sea oscura.

Si los argumentos agrupados tienen un significado en sí mismos, entonces esa capa de complejidad ayuda a comprender todo el diseño: en su lugar, llámelo capa de abstracción.

Puede encontrar que, en lugar de una docena de argumentos individuales o una gran matriz, el mejor diseño es con dos o tres argumentos cada uno agrupando datos correlacionados.

Mouviciel
fuente
1

En su caso, preferiría el método de su colega. Si estabas escribiendo modelos y yo estaba usando tus modelos para desarrollarlos. Veo la firma del método de su colega y puedo usarlo de inmediato.

Mientras tanto, tendría que pasar por la implementación de su searchQueryfunción para ver qué parámetros espera su función.

Preferiría su enfoque solo en el caso de que searchQueryse limite a buscar solo dentro de una sola tabla, por lo que no habrá uniones. En ese caso, mi función se vería así:

function searchQuery($params = array()) {
    foreach($params as $param => $value) {
        $query->where($param, $value);
    }
} 

Entonces, inmediatamente sé que los elementos de la matriz son en realidad los nombres de columna de una tabla en particular que la clase que tiene este método representa en su código.

Ozair Kafray
fuente
1

Haz ambas cosas, más o menos. array_mergepermite una lista explícita en la parte superior de la función, como le gusta a su colega, mientras evita que los parámetros se vuelvan difíciles de manejar, como prefiera.

También sugiero usar la sugerencia de @ chiborg de los comentarios de la pregunta: es mucho más claro lo que pretendes.

function searchQuery($params = array()) {
    $defaults = array(
        'name' => '',
        'phone' => '',
        ....
    );
    $params = array_merge($defaults, $params);

    if(!empty($params['name'])) {
        $query->where('name', $params['name']);
    }
    if (!empty($params['phone'])) {
        $query->join('phone');
        $query->where('phone', $params['phone']);
    }
    ....
}
Izkata
fuente
0

También podría pasar una cadena que se asemeje a una cadena de consulta y usarla parse_str(porque parece que está usando PHP, pero otras soluciones probablemente estén disponibles en otros idiomas) para procesarla en una matriz dentro del método:

/**
 * Executes a search in the DB with the constraints specified in the $queryString
 * @var $queryString string The search parameters in a query string format (ie
 *      "foo=abc&bar=hello"
 * @return ResultSet the result set of performing the query
 */
function searchQuery($queryString) {
  $params = parse_str($queryString);
  if (isset($params['name'])) {
    $query->where('name', $params['name']);
  }
  if (isset($params['phone'])) {
    $query->join('phone');
    $query->where('phone', $params['phone']);
  }
  ...

  return ...;
}

y llámalo como

$result = searchQuery('name=foo&phone=555-123-456');

Puede usar http_build_querypara convertir de una matriz asociativa a una cadena (lo contrario que parse_strhace).

Carlos Campderrós
fuente