Función generalizada de consulta SQL -- php camp codereview Relacionados El problema

Generalized Sql Query Function


0
vote

problema

Español

Una cosa que me di cuenta Mientras pasas por el código ayer en mi pasantía es que ejecuto las consultas de SQL con bastante frecuencia. Así que decidí escribir una función para mantener todas las llamadas en un solo lugar y reducir el número de líneas de código. No cubre todas las llamadas SQL posibles, pero cubre las llamadas básicas que creo.

Entonces, mi pregunta es, ¿existen prácticas de codificación que pueda mejorar o haber violado mientras escribo esto?

Editar: Rompí la función en cuatro funciones separadas. Cuando se trata de PDO, mis jefes dijeron que hicieron eso detrás de las escenas con cada $ DB_Function (hay una secuencia diferente para cada servidor que acceden) que se llama. Simplemente estoy preparando la cadena que manipulan. También siempre he visto matrices asociativas como pares de valor de campo, por lo que doy la bienvenida a cualquier sugerencia en nombres alternativos.

      <?      *  INPUT:      *  (string) Db_function: Which sql function to call      *  (string) Table: The table to use      *  (string/array) Fields: The values to be manipulated along with valid sql statements/modifiers(e.g DISTINCT or aliases) ,       *                         its a string for SELECT/DELETE or an array for INSERT/UPDATE.      *  (string) Conditional: Statements that go at the end of the query such as WHERE, ORDER BY, JOIN etc.      *        *  OUTPUT:      *          TRUE for SELECT/DELETE or the query results for INSERT/UPDATE */      function sql_Select($Db_Function,$Table, $Fields, $Conditional = NULL){          $sql = "SELECT $Fields FROM $Table";            if(!is_null($Conditional)){                 $sql .= $Conditional;                 return $Db_Function($sql,1,1,1);             }             return $Db_Function($sql,1,1,1);          }      function sql_Update($Db_Function,$Table, $Fields, $Conditional = NULL){          $sql = "UPDATE $Table SET";               foreach($Fields as $field => $value){                 $sql .= is_numeric($value) ? " $field = $value , " : " $field = '$value' , " ;                 }                 $sql = preg_replace('/,$/', '', trim($sql)); //Removes the extra ','            if(!is_null($Conditional)){             $sql .= $Conditional;             return $Db_Function($sql,0,0,0);             }            return $Db_Function($sql,0,0,0);       }      function sql_Insert($Db_Function,$Table, $Fields, $Conditional = NULL){          $Field_Name = implode(',',array_keys($Fields));         $sql = "INSERT INTO $Table ($Field_Name) VALUES ( ";          foreach($Fields as $field => $value){                         $sql .= is_numeric($value) ? " $value, " : " '$value', " ;                      }                     $sql = preg_replace('/,$/', '', trim($sql)); //Removes the extra ','                      $sql .= ')';           if(!is_null($Conditional)){             $sql .= $Conditional;             return $Db_Function($sql,0,0,0);             }            return $Db_Function($sql,0,0,0);     }      function sql_delete($Db_Function,$Table, $Fields, $Conditional = NULL){          $sql = "DELETE FROM $Table";            if(!is_null($Conditional)){                 $sql .= $Conditional;                 return $Db_Function($sql,0,0,0)             }             return $Db_Function($sql,0,0,0)          }      ?>   
Original en ingles

One thing I realized while going through code yesterday at my internship is that I run sql queries quite often. So I decided to write a function to keep all of the calls in one place and reduce the number of lines of code. It doesn't cover all possible sql calls but it does cover the basic calls I believe.

So my question is, are there any coding practices I could improve upon or have violated while writing this?

EDIT: I broke the function up into four separate functions. When it comes to PDO my bosses said they did that behind the scenes with each $db_function(there is a different sequence for each server they access) that is called. I'm simply preparing the string that they manipulate. Also I've always viewed associative arrays as field value pairs so I welcome any suggestions in alternative names.

    <?      *  INPUT:      *  (string) Db_function: Which sql function to call      *  (string) Table: The table to use      *  (string/array) Fields: The values to be manipulated along with valid sql statements/modifiers(e.g DISTINCT or aliases) ,       *                         its a string for SELECT/DELETE or an array for INSERT/UPDATE.      *  (string) Conditional: Statements that go at the end of the query such as WHERE, ORDER BY, JOIN etc.      *        *  OUTPUT:      *          TRUE for SELECT/DELETE or the query results for INSERT/UPDATE */      function sql_Select($Db_Function,$Table, $Fields, $Conditional = NULL){          $sql = "SELECT $Fields FROM $Table";            if(!is_null($Conditional)){                 $sql .= $Conditional;                 return $Db_Function($sql,1,1,1);             }             return $Db_Function($sql,1,1,1);          }      function sql_Update($Db_Function,$Table, $Fields, $Conditional = NULL){          $sql = "UPDATE $Table SET";               foreach($Fields as $field => $value){                 $sql .= is_numeric($value) ? " $field = $value , " : " $field = '$value' , " ;                 }                 $sql = preg_replace('/,$/', '', trim($sql)); //Removes the extra ','            if(!is_null($Conditional)){             $sql .= $Conditional;             return $Db_Function($sql,0,0,0);             }            return $Db_Function($sql,0,0,0);       }      function sql_Insert($Db_Function,$Table, $Fields, $Conditional = NULL){          $Field_Name = implode(',',array_keys($Fields));         $sql = "INSERT INTO $Table ($Field_Name) VALUES ( ";          foreach($Fields as $field => $value){                         $sql .= is_numeric($value) ? " $value, " : " '$value', " ;                      }                     $sql = preg_replace('/,$/', '', trim($sql)); //Removes the extra ','                      $sql .= ')';           if(!is_null($Conditional)){             $sql .= $Conditional;             return $Db_Function($sql,0,0,0);             }            return $Db_Function($sql,0,0,0);     }      function sql_delete($Db_Function,$Table, $Fields, $Conditional = NULL){          $sql = "DELETE FROM $Table";            if(!is_null($Conditional)){                 $sql .= $Conditional;                 return $Db_Function($sql,0,0,0)             }             return $Db_Function($sql,0,0,0)          }      ?> 
  
   
   

Lista de respuestas

3
 
vote

Argggh, mis ojos se queman!

inyectables sql

Debe entender las inyecciones de SQL. Es probable que este código promueva las inyecciones de SQL. Lea las declaraciones preparadas. Parece que probablemente estés pasando en una función 99887766665544334 como una cadena. Esto le impide utilizar declaraciones preparadas.

por favor, no use mysql_* Funciones en nuevo código < / a>. Ya no están mantenidos y la proceso de deprecación ha comenzado en ello. Consulte la caja roja ? Conozca acerca de declaraciones preparadas en su lugar, y use pdo o mysqli - Este artículo te ayudará a decidir cuál. Si elige PDO, Aquí hay un buen tutorial .

one talla se ajusta a ninguno

Su función es demasiado compleja. Debe dividirse en métodos para cada acción. Esto eliminará la necesidad del interruptor grande y si las declaraciones y lo harán más legible. Su código será más fácil de probar. Esto también hace que tenga una llamada de función variable $Db_Function($sql, 0, 0, 0); . Es fácil tener errores llamando a funciones inexistentes con esto.

Respuesta Para editar: se ve mejor con funciones separadas ahora.

Methodon Naming

Realmente tienes un esquema de nombramiento horrible. ¿Te dicta esto por tus jefes? Personalmente, ya creo que son tontos por querer una clase como esta que promoverían las inyecciones de SQL. ¿Tienes un nombre de pascal mixto aaaa_Bbbb_Cccc ? Las opciones SANE son camelCase8 , UpperCamelCase , --channel0 , 99887776655443311 .

Los métodos no deben tener 99887776655443312 que los precedieron. Su uso debe ser así:

  --channel3  

Debe ser obvio en el objeto que cree que tiene un objeto de tipo SQL. El --channel4 Frente a los nombres de los métodos y luego se interpone en el camino.

parámetro no utilizado

El método --channel5 no usa el parámetro 99887766655443316 y, por lo tanto, debe eliminarse.

Números mágicos

MAGIC --channel7 's y --channel8 ' s son un signo de mal diseño.

consistencia

Encuentre una forma consistente de nombrar variables. --channel9 , parser.add_argument('-c', '--channel', default='all', help='channel/s to hop (i.e. 3 or 3,6,9 or 3-14 or all or 0 ' 'for current channel') 020 , 99887766555443321 está en la minoría. No me gusta Pascal_Case para las variables en PHP, pero la inconsistencia es aún peor.

 

Argggh, my eyes burn!

SQL Injections

You should understand SQL injections. This code is likely to promote SQL injections. Read up about prepared statements. It looks like you are probably passing in a mysql_* function as a string. This stops you from using prepared statements.

Please, don't use mysql_* functions in new code. They are no longer maintained and the deprecation process has begun on it. See the red box? Learn about prepared statements instead, and use PDO or MySQLi - this article will help you decide which. If you choose PDO, here is a good tutorial.

One Size Fits None

Your function is overly complex. It should be broken into methods for each action. This will remove the need for the big switch and if statements and make it more readable. Your code will be easier to test. This also causes you to have a variable function call $Db_Function($sql, 0, 0, 0);. It is easy to have errors by calling non-existent functions with this.

Response To Edit: It looks better with separate functions now.

Method Naming

You really have an awful naming scheme. Is this dictated to you by your bosses? Personally I already think they are silly for wanting a class like this that would promote SQL Injections. You have a mixed pascal case name aaaa_Bbbb_Cccc? Sane choices are camelCase, UpperCamelCase, Pascal_Case, lower_pascal_case.

The methods should not have sql_ preceding them. Their usage should be like this:

$sql = new sql; // Or whatever your class is called. $sql->insert(/* params go here */); 

It should be obvious from the object that you create that you have an sql type object. The sql_ in front of the method names then just gets in the way.

Unused Parameter

The sql_delete method does not use the $Fields parameter and it should therefore be removed.

Magic Numbers

Magic 0's and 1's are a sign of poor design.

Consistency

Please find a consistent way to name variables. $sql, $fields, $value are in the minority. I dislike Pascal_Case for variables in PHP, but inconsistency is even worse.

 
 
   
   
1
 
vote

Incluso con los cambios, este nuevo código aún cae debajo de todas las categorías que Pablo publicó anteriormente. Además de esto, agregaría los problemas adicionales siguientes:

  • ¿Cómo es este código testable?

  • Si aparece una nueva persona, ¿cómo es tan fácil de entender? ¿Verdadero para seleccionar / Eliminar o los resultados de la consulta para insertar / actualizar? La lógica como esta me confundiría enormemente.

  • Esta es una forma muy php 4ish de resolver un problema. ¿Ha considerado avanzarse hacia un enfoque de OOP?

 

Even with the changes, this new code still falls underneath all of the categories that Paul posted earlier. On top of this, I would add the extra following problems:

  • How is this code testable?

  • If a new person comes along, how is this easy to understand? TRUE for SELECT/DELETE or the query results for INSERT/UPDATE? Logic like this would confuse me immensely.

  • This is a very PHP 4ish way of going about solving a problem. Have you considered moving towards an OOP approach?

 
 

Relacionados problema

28  Biblioteca PHP AutoOoGer  ( Php autoloader library ) 
Básicamente, había escrito esta clase hace poco tiempo para aliviar la carga automática de nuestras bibliotecas locales. La premisa es que todo se divide po...

30  Codeigniter Active Record Subqueries  ( Codeigniter active record subqueries ) 
Yo uso CodeIgNiter en el trabajo, y uno de nuestros archivos modelo tuvo muchas subcarías en ella. Originalmente tuve que escribir manualmente cada subconsu...

2  PHP Prueba ISSET e instantánea en un disparo. Posible en php?  ( Php test isset and instantiate in one shot possible in php ) 
¿Cómo mejorar este tipo de redundancia? 'done'8 Quiero evitar repetir $ _bet ['NID'] Ejemplo de uso útil de uso: 'done'9 ...

1  Nombre de la bahía Pirata y MAILER MAILER  ( Pirate bay name and magnet mailer ) 
Entonces, primera vez que trato PHP. Pensé que me gustaría establecer un objetivo para mí, dividirlo en problemas más pequeños y comenzar a andar en Google. A...

10  Comparando datos en 2 tablas en diferentes servidores con suma de comprobación  ( Comparing data in 2 tables on different servers with checksum ) 
Así que he tenido un problema en el que necesito comparar datos en 2 tablas diferentes en dos servidores diferentes. Ahora, sé que MySQL admite CHECKSUM TABL...

41  Conexión de la base de datos en constructor y destructor  ( Database connection in constructor and destructor ) 
Estoy jugando con diferentes maneras de hacer la interacción de la base de datos en PHP, y una de las ideas con las que he estado jugando está conectando a la...

-4  Demasiado anidamiento [cerrado]  ( Too much nesting ) 
cerrado. Esta pregunta es off-topic . Actualmente no está aceptando respuestas. ¿Quieres ...

2  PHP - Función para manejar las solicitudes  ( Php function to handle requests ) 
Tengo la siguiente función que maneja las solicitudes. Tiene más de 130 líneas public function run() { $objectRequests = json_decode(file_get_contents(...

2  Formulario básico de comentarios de PHP  ( Basic php comment form ) 
Soy un novato de programación. He escrito este simple script PHP para ejecutar un formulario de comentarios muy básico y apreciaría cualquier comentario, espe...

4  Consulta para trazar un histograma de calificaciones de canciones  ( Query to plot a histogram of song ratings ) 
Escribí código para trazar una distribución de calificación, que se parece a esto: La función más importante es el stats Función: function stats($so...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos