Try it with the some_id DB key format and see what happens for you. It didn’t work for me so I just changed the key to id to get the demo done. I did keep the hidden name id so every form did not have different hidden code.
Where I set it was like this…
$db = new DatabaseTable($pdo, 'contacts', 'contact_id');
I think you misunderstand what’s happening. The hidden code is just the value for the primary key. The name of the form field has to match the name of the column (The same is true for all form fields).
I don’t know where you are in the book but at a potential later point names the form fields are are wrapped in an array <input type="text" name="joke[joketext]" /> and joketext matches the column name. The same is true of joke[id].
As I mentioned earlier, ultimately you can get to a point where you don’t any code specific for a particular form unless special logic is required:
class FormController {
private $databaseTable;
private $dataKey;
private $formTemplate;
public function __construct(\Ninja\DatabaseTable $databaseTable, $dataKey, $formTemplate) {
$this->databaseTable = $databaseTable;
$this->dataKey = $dataKey;
$this->formTemplate = $formTemplate;
}
public function display() {
//You probably don't want to use the superglobal here but pass it as an argument
//For ease of understanding the demo here's
$id = $_GET['id'];
$data = $id ? $this->dataBaseTable->findById($id) : null;
//and again, you'd abstract the logic for loading/rendering templates but you get the idea
require $this->templateFile;
}
public function submit() {
//You probably don't want to use the superglobal here but pass it as an argument
//For ease of understanding the demo here's
$this->databaseTable->save($_POST[$this->dataKey]);
}
}
$jokeFormController = new FormController(new DatabaseTable('joke'), 'joke', 'jokeform.tpl.php');
$categoryFormController = new FormController(new DatabaseTable('category'), 'category', 'categoryform.html.php');
And now you have reusable code that works for any form. You’re going to ask about validation:
interface FormValidate {
public function validate($date): array;
}
class FormController {
private $databaseTable;
private $dataKey;
private $formTemplate;
private $validation
public function __construct(\Ninja\DatabaseTable $databaseTable, FormValidate $validation, $dataKey, $formTemplate) {
$this->databaseTable = $databaseTable;
$this->dataKey = $dataKey;
$this->formTemplate = $formTemplate;
}
public function display($errors = []) {
//You probably don't want to use the superglobal here but pass it as an argument
//For ease of understanding the demo here's
$id = $_GET['id'];
$data = $id ? $this->dataBaseTable->findById($id) : null;
//and again, you'd abstract the logic for loading/rendering templates but you get the idea
//template file now also has access to the $errors array for display purposes
require $this->templateFile;
}
public function submit() {
//You probably don't want to use the superglobal here but pass it as an argument
//For ease of understanding the demo here's
$data = $_POST[$this->dataKey];
$errors = $this->validation->validate($data);
if (empty($errors)) {
$this->databaseTable->save($data);
}
else {
$this->display($errors);
}
}
}
And there you have a controller that can be used for almost any basic form. Yes there will be edge cases where you need additional logic but the validator can have dependencies to handle “That username is already in use” type errors.
Want a new form? Just create the template and an instance of the controller
$authorFormController = new FormController(new DatabaseTable('author'), 'author', 'authorform.html.php');
No, I get that the hidden id is the value. I also read the part about the form names being an array, which is one of the new things I learned so as to not deal with the submit. In an older script I just popped the array to get rid of it but I like the approach you show in the book. You will see that the sample form I posted does in fact utilize your technique. I will want to do a comparison of the two options later.
I ended up doing a steady read starting at chapter 8 since everything previous was all stuff I already knew. Hopefully I didn’t miss anything important before that. I will go back to the start when I get to the end. And yeah, I did catch the beginning part about skipping around.
I expect to reach Ninja status in the next couple days. I have considered myself a Master Coder long enough. I am ready for my Ninja belt Sensei. 
You know, I thought about what you pointed out about where the id appears in the POST array. The caller should not have to care where in the array the id is. By using array_pop as I did in the modified insert method I posted it dictates that the id must be the last element or it will break. Well, that’s no good now is it? Allow me present a better modification which still does the two things I wanted that I mentioned before. It allows me to use name_id naming for the primary key and it generates a “clean” query, meaning it doesn’t update the key with the same exact value.
public function update($fields) {
$id = $fields['id']; // benanamen mod
unset($fields['id']);// benanamen mod
$query = ' UPDATE `' . $this->table .'` SET ';
foreach ($fields as $key => $value) {
$query .= '`' . $key . '` = :' . $key . ',';
}
$query = rtrim($query, ',');
$query .= ' WHERE `' . $this->primaryKey . '` = :primaryKey';
//Set the :primaryKey variable
$fields['primaryKey'] = $id;// benanamen mod
$fields = $this->processDates($fields);
$this->query($query, $fields);
}
@TomB, I am going to try and address things you have brought up as time allows. You brought up about cyclomatic complexity so lets take a look at it. If we were to decide the path to take based on your complexity argument, my proposed solution is LESS complex for both an insert AND update using your unmodified class methods.
My Update Complexity Rating 5
complexity 2: My Ternary
complexity 2 ‘update’
complexity 1 ‘query’My Insert Complexity Rating: 6
complexity 2: My Ternary
complexity 3 ‘insert’
complexity 1 ‘query’Your Update complexity Rating: 6
complexity 0 $joke->save($_POST[‘joke’]);
complexity 3 ‘save’
complexity 2 ‘update’
complexity 1 ‘query’Your Insert complexity Rating: 7
complexity 0 $joke->save($_POST[‘joke’]);
complexity 3 ‘save’
complexity 3 ‘insert’
complexity 1 ‘query’
MY TOTAL COMPLEXITY SCORE: (11)
YOUR TOTAL COMPLEXITY SCORE: (13)
The point which you’re missing again, is that you need to duplicate the logic for each form.
Regardless, function calls care not measured by cyclomatic complexity. Cyclomatic complexity is a measure of logic: If statements, declared functions, loops, etc. linearly independent paths through the code.
This code:
public function submit() {
$table->save($_POST['whatever']);
}
Has a cyclomatic complexity of 1, the function definition.
This code:
public function submit() {
isset($_POST['data']['id']) ? $db->update($_POST['data']) : $db->insert($_POST['data']);
}
Has a cyclomatic complexity of 2 (1 if statement + 1 function definition). This is repeated for each form you create.
And because this logic might be different for each form, needs to be repeated for each form.
2 forms = 4 complexity, 3 forms = 6 complexity, etc. Using the save function: 2 forms = 2 complexity, 4 forms = 4 complexity. And if you used my example above: 10000 forms = 1 complexity because the whole controller is reusable.
Why is this an issue? What negative effect does this have?
Really, if you care that much to solve this you just want to use the code:
private function update($fields) {
//read the primary key value out of the record
$pk = $fields[$this->primaryKey];
//Remove the primary key from the generated SET clause
unset($fields[$this->primaryKey]);
$query = ' UPDATE `' . $this->table .'` SET ';
foreach ($fields as $key => $value) {
$query .= '`' . $key . '` = :' . $key . ',';
}
$query = rtrim($query, ',');
$query .= ' WHERE `' . $this->primaryKey . '` = :primaryKey';
//Write the primary key value back into `primaryKey` for the WHERE clause
$fields['primaryKey'] = $pk;
$fields = $this->processDates($fields);
$this->query($query, $fields);
}
Because then you don’t have to worry about having extra fields on the form.
Though the additional logic is probably a bigger overhead than sending the primary key to MySQL in both the SET and WHERE clauses.
Just personal preference. It’s no different than doing an update where you didn’t change some of the fields. They still update the same exact data.
EDIT:
Yeah, you got a point there.
Actually, it doesn’t. Try updating a record with the exact same values and read $pdo->rowCount(), it’ll show zero because mysql is smart enough to to issue a disk write when the old/new values are the same.
Not if you changed at least thing. If you didn’t change anything, then yes.
My point is, even if you send id = 4 back to the database, it won’t actually update the column for the record as it recognises the values are the same, it will only update fields that have changed.
I see what your saying. The query proves it out so it doesn’t matter that the id is in the set.
That’s not going to work. If you remember, I wanted to use the same hidden field name “id” on all forms. Therefore, trying to get the value from $pk = $fields[$this->primaryKey]; will not work since $this->primaryKey actually holds “contact_id” instead of “id” which is what would be in $fields array. I would have to do it the way I showed.
why give yourself extra work? You still need the PK form field anyway, why add id as well?
Then every form would need a different hidden field name. Not that big a deal. If I named the hidden field contact_id then it would be OK.
I’m just not sure I see the logic. Every other form field is required to match the name of the column, why would the primary key be any different?
Copy/Paste mentality. LOL! There is no logic.
MySQL yes, but databases that employ MVCC, like Postgres, will do the update physically on disk even if no values have changed. That’s because every update is an insert under the hood, therefore PDO will always return 1 as the number of affected rows. Therefore, a better general logic for portable code would be to avoid such updates if possible. In many cases this is not a problem because autovacuum will take care of garbage collection but if such updates are very frequent they may lead to unnecessary disk space usage and may require additional actions to claim it back.