Hi, what I'm trying to do is an UPDATE in one of my database tables, I know I'm not doing it right because of how I have it structured since INSERT INTOs work fine for me, but the tutorial I'm following doesn't talk much about the UPDATE , so I'm doing it as follows:
<?php
require "Conexion.php";
class ModeloCerrarCaja{
static public function mdlCerrarCaja($tabla,$Hora_Cierre,$Total_Ventas,$Estado,$cajaAbierta,$Sucursal){
if($cajaAbierta!= null){
$stmt = Conexion::conectar()->prepare("UPDATE $tabla SET Hora_Cierre = :$Hora_Cierre, Total_Ventas = :$Total_Ventas, Estado = :$Estado WHERE Sucursal = :$Sucursal AND caja_sucursal = :$cajaAbierta");
$stmt -> bindParam(":Hora_Cierre", $Hora_Cierre, PDO::PARAM_STR);
$stmt -> bindParam(":Total_Ventas", $Total_Ventas, PDO::PARAM_STR);
$stmt -> bindParam(":Estado", $Estado, PDO::PARAM_STR);
if($stmt->execute()){
return "ok";
}else{
return "error";
}
}else{
$stmt = Conexion::conectar()->prepare("SELECT * FROM $tabla");
$stmt -> execute();
return $stmt -> fetchAll();
}
$stmt -> close();
$stmt = null;
}
}
In the same way, if you could give me a basic guide to learn how to do this syntax correctly or a tutorial, I would greatly appreciate it.
You need to understand that markers are put into prepared queries, not values .
So this is wrong:
Precisely, the crux of a prepared query is to put a marker in the place where each data goes and then, with an appropriate method (
bindParam
in this case) you pass the value for each data associated to its marker. It's like telling the controller: where I put this marker, you must put this data, but check that there is no risk of injection .With that understood, your code should look like this:
Simply, we have removed all the
$
, thus having what is required, which are bookmarks .Now, you must tell the DBMS to which data each marker that you put above is associated , something VERY IMPORTANT is that you must write each marker exactly the same as you wrote it in the SQL statement, and you must ensure that you associate in each case the corresponding value and not another value . Therefore, if you are receiving these variables in parameter:
You should do the bindings using those variables :
You weren't linking
:Sucursal
or:caja_sucursal
. I have assumed that they are also strings, if one is not, change itPDO::PARAM_STR
to the correct data type.That's how it should work, barring other bugs.
Bad Practice Warning
In your method, these two lines at the end will never be executed :
Whyeeee if I have put them with very good intention? Well, because you have
return
more above and nothing that is afterreturn
it is executed.Also, there are two redundant lines, one would suffice (in fact, in some scenario, I don't remember now if with mysqli or with PDO), calling
close()
didn't destroy the object,null
instead it always destroyed it.Perhaps a better way to structure your method would be this:
If you notice, an object has been used
$mData
to control the internal logic of the method. Returning a keysuccess
set totrue
when there is some data orfalse
when there is some error.Since this method can return two types of successful responses, I have put a key
action
by which you will be able to distinguish if oneupdate
or one occurredread
and attach to the response: show the message of number of rows updated for example, or read the datafetchAll
you got from in theSELECT
.As for the cases of error, when
success
it isfalse
the message of what happened, it will be found in a keymsg
.