*) All your radio buttons have the same id attribute. It should be unique.
*) Give the id "addBoard" to the submit button.
*) Why do you use POST and GET?
$boardID = $_POST['option'];
$postID = $_GET['id'];
*) You missed the $ sign in statement->bindValue(":boardID",boardID);!
*) Posted data type should correspond to db data type. Use a third parameter in bindValue() to define the corresponding data type. And use a function:
private function getInputParameterDataType($value) {
$dataType = PDO::PARAM_STR;
if (is_int($value)) {
$dataType = PDO::PARAM_INT;
} elseif (is_bool($value)) {
$dataType = PDO::PARAM_BOOL;
}
return $dataType;
}
And then call like this:
$statement->bindValue(":boardID",$boardID, $this->getInputParameterDataType($boardID));
*) PDO::prepare() can throw a PDOException OR the value FALSE. So you should handle both cases. I wrote an answer about this: My answer for exception handling of prepare() & execute().
*) Have you made some changes until now? Works everything, or not yet?
Ok, I'll study it now.
Solutions for the new code:
*) Don't use semicolon ";" at the end of sql statements.
*) Update should have this form:
UPDATE [table-name] SET [col1]=:[col1],[col2]=:[col2] WHERE [PK-name]=:[PK-name]
*) For readability: pass the sql statement in a variable and use points to delimit used variables in it. Like:
$sql = "UPDATE board SET postID=:" . $postid . " WHERE boardID=:boardID"
$statement = $conn->prepare($sql);
*) As @teresko reminded you: you didn't pass $boardID to savePostToBoard():
$sBoard = $sB->savePostToBoard($postID, $boardID);
public function savePostToBoard($postID, $boardID) {...}
*) Use either $postID or $postid overall. Right now you are using both forms. Make a choice.
It should work now. Let me know.
Some recommandations:
*) Let your methods do only one thing (if possible). In your question, the connection creation doesn't belong in the method. If you call 20 methods which require a connection, then you have to write the same connection creation code in each of them. In your case, better define a getConnection() method in the Board class.
public function getConnection() {
$conn = Db::getInstance();
$conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$conn->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
return $conn;
}
And now you have only this in your savePostOnBoard() by calling it:
public function savePostToBoard($postID) {
$statement = $this->getConnection()->prepare(...);
//...
}
*) In general, it's better to use the constructor for passing the variables used later by the methods. That's the role of the constructor: to initialize, e.g to give initial values to the object properties on creation. Example:
class Auto {
private $color;
private $doors;
private $airConditioning;
public __function __construct($color = 'blue', $doors = 3, $airConditioning = true) {
$this->color = $color;
$this->doors = $doors;
$this->airConditioning = $airConditioning;
}
}
$myAuto = new Auto('red', 4, false);
$hisAuto = new Auto('yellow', 8, true);
Oh, and always give easy-to-follow names to variables, functions, classes, etc. In your case, the upper phrase and this one would apply, for example, like this:
$board = new Board($boardID);
$boardUpdated = $board->update($postID);
See? Nicer names, more logical (following our real-world perception) arrangement of the arguments.
*) I would also recommend you to split your code in methods. This way you achieve a better reusability of code pieces and an elegant, easy-to-follow structure. Maybe something like this in analogy with your code:
public function savePostToBoard($postID, $boardID) {
$sql = "UPDATE board SET postID=:" . $postID . " WHERE boardID=:boardID";
$bindings = array(
':postID' => $postID,
':boardID' => $boardID
);
$statement = $this->execute($sql, $bindings);
return $statement->rowCount();
}
protected function execute($sql, array $bindings = array()) {
$statement = $this->prepareStatement($sql);
$this->bindInputParameters($statement, $bindings);
$this->executePreparedStatement($statement);
return $statement;
}
private function prepareStatement($sql) {
$statement = $this->getConnection()->prepare($sql);
return $statement;
}
private function bindInputParameters($statement, $bindings) {
foreach ($bindings as $key => $value) {
$statement->bindValue(
$this->getInputParameterName($key)
, $value
, $this->getInputParameterDataType($value)
);
}
return $this;
}
private function getInputParameterName($key) {
if (is_int($key)) {
return $key + 1;
}
$trimmed = ltrim($key, ':');
return ':' . $trimmed;
}
private function getInputParameterDataType($value) {
$dataType = PDO::PARAM_STR;
if (is_int($value)) {
$dataType = PDO::PARAM_INT;
} elseif (is_bool($value)) {
$dataType = PDO::PARAM_BOOL;
}
return $dataType;
}
private function executePreparedStatement($statement) {
$statement->execute();
return $this;
}
Good luck with your project!
