Update MySql DB using Classes

[MutatedVirus]

Hey all, How would I make it so this code will update the database?

In the class(name is News) I have this function:

public function save($isNewNews = false) {
        $db = new DB();
        if(!$isNewNews) {
            $data = array(
                "Title" => "'$this->Title'",
                "descrip" => "'$this->descrip'",
                "Content" => "'$this->Content'",
                "Image" => "'$this->Image'",
                "colour" => "'$this->Image'",
                "tags" => "'$this->tags'"
            );
            
            $db->update($data, 'news', 'id = '.$this->id);
        }else {
            $data = array(
                "Title" => "'$this->Title'",
                "descrip" => "'$this->descrip'",
                "Content" => "'$this->Content'",
                "colour" => "'$this->colour'",
                "Image" => "'$this->Image'",
                "tags" => "'$this->tags'",
                "date" => "'".date("Y-m-d H:i:s")."'"
                    
                
            );
            
            $this->id = $db->insert($data, 'news');
        }
        return true;
    }
    

To add a new row to the DB i call
$foo = new News($data);
$foo = save(true);


If i try $foo = save(false);

It returns a MySql error

[vinny42]

 

It returns a MySql error

 

It seems logical that you'd post the error so when can see it and tell you what the problem could be...

[MutatedVirus]

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '' at line 1

[vinny42]

Ok, my mistake, it would ofcourse also help to know what the query was.

[MutatedVirus]

It's just an update query linked into a DB class file

DB.Inc.php update
 

 public function update($data, $table, $where) {
        foreach ($data as $column => $value) {
            $sql = "UPDATE $table SET $column = $value WHERE $where";
            mysql_query($sql) or die(mysql_error());
        }
        return true;
    }

I dont know if this ill help but it might

The php im using to update

 

if(isset($_POST['submit']))
{
    $Title = $_POST['Title'];
    $Content = $_POST['Content'];
    $colour = $_POST['colour'];
    $Image=($_FILES['Image']['name']);
    $tags = $_POST['tags'];
    $target = './news_imgs/';
    $target = $target . basename( $_FILES['Image']['name']);
    $success = true;
    var_dump($target);
    
    if($success)
    {
        $data['Title'] = $Title;
        $data['descrip'] = $Content; 
        $data['Content'] = $Content;
        $data['colour'] = $colour;
        $data['Image'] = $Image;
        $data['tags'] = $tags;
        $newNews = new News($data);

        $newNews->save(false);
        


if(move_uploaded_file($_FILES['Image']['tmp_name'], $target))
       {

            //Tells you if its all ok
            echo "The file ". basename( $_FILES['uploadedfile']['name']). 
                 " has been uploaded, and your information has been added to the directory";
            }
            else {

            //Gives and error if its not
            echo "Sorry, there was a problem uploading your file.";
            die();
       }
echo "1 record added";
header("Location: ./news.php");
    }

if (!mysql_query($update))
  {
  die('Error: ' . mysql_error());
  }
echo "1 record added";
header("Location: ./news.php");

}
?>

[mac_gyver]

your error reporting logic (the die() statement) needs to also display the query statement, in $sql, so that you can see what the query actually is (the id is likely empty.)

 

you should not run queries inside of loops. your UPDATE query should update all the supplied columns/values in one query.

 

also, once you fix the current problem, you should NOT be creating an instance of your db class every time you call one of your news methods. you are figuratively killing your database server by opening a connection on each method call and closing it when that method call ends. since your news class is dependent on the database class to function at all, you should create one instance of your database class in your main code and use dependency injection to make that one instance of the database class available inside the news class.

[MutatedVirus]

Okay thanks Thanks helped me