Checking for all query parameters before inserting into a database

Question

The code works as it is but it's very messy. Is there a way to clean up the null checking in the if statement?

public function submitCoil ($data) {
 $db = Database::getInstance();
 //Define certain query values
 $uniqueID = $this->hash("id");
 $queryData = array(
 'fields' => array(
 'uniqueid',
 'uploaded',
 'name',
 'resistance',
 'wrapped',
 'category',
 'description',
 'wraps',
 'images',
 'wire_one',
 'wire_two',
 'wire_three',
 'wire_four',
 'wire_five',
 'wire_six'
 ),
 'values' => array(
 'uniqueID' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
 )
 );
 //Make sure mandatory datas are filled out
 if (!empty($queryData['values']['name']) && !empty($queryData['values']['resistance']) && !empty($queryData['values']['wrapped']) && !empty($queryData['values']['description']) && !empty($queryData['values']['wraps'])) {
 $db->insertFew("coils", $queryData['fields'], $queryData['values']);
 } else {
 echo "Please fill all fields";
 } 

• BCdotWEB: As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. For examples of good titles, check out Best of Code Review 2014 - Best Question Title Category You may also want to read How to get the best value out of Code Review - Asking Questions.
• AD7six: @BCdotWEB except for the winner/accepted answer that first link is IMO not a list of good question titles. They are good examples of on-topic humor - which are enticing but give no real indication of what the question is about.
• BCdotWEB: @AD7six It's a standard comment copy-pasted from here: meta.codereview.stackexchange.com/a/4957/10582
• AD7six: Coolio - my sentiment stands though.

Answer 1

Use a validation function

There are many possible solutions, one thing you can do is use a separate function to validate your data:

public function submitCoil($data)
{
 ...
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 $return = true;
 foreach ($mandatoryFields as $field) {
 if (empty($values[$field])) {
 // Consider doing something like this so you can know/say
 // what the problem is.
 // $this->validationErrors[$field] = "This is empty";
 $return = false;
 }
 }
 return $return;
}

This function simply loops on all the keys that you specify as needing to be not-empty and returns true only if they are all populated.

Many simple functions

Always strive to have simple functions, that do one thing - for example:

public function submitCoil($data)
{
 $queryData = $this->translateData($data);
 if (!$this->validates($queryData['values'])) {
 return false;
 }
 return $this->insert($queryData);
}
protected function translateData($data)
{
 $uniqueID = $this->hash("id");
 return [
 'fields' => [
 'uniqueid',
 'uploaded',
 'name',
 'resistance',
 'wrapped',
 'category',
 'description',
 'wraps',
 'images',
 'wire_one',
 'wire_two',
 'wire_three',
 'wire_four',
 'wire_five',
 'wire_six'
 ],
 'values' => [
 'uniqueID' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
 ]
 ];
}
protected function validates($values)
{
 $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];
 foreach ($mandatoryFields as $field) {
 if (empty($values[$field])) {
 return false;
 }
 }
 return true;
}
protected function insert($data)
{
 $db = Database::getInstance();
 return $db->insertFew("coils", $data['fields'], $data['values']);
}

This allows the building-blocks to be changed - or used individually. It also aides readability/maintainability as it is much clearer at a glance what a function does (assuming methods are well-named).

Avoid repetition

The fields and array keys for the values key in $queryData look to serve the same purpose - although they are different. You don't need 'fields' if the keys can be used i.e.:

$queryData = [
 'uniqueid' => $uniqueID,
 'uploaded' => date("F j, Y", time()),
 'name' => $data['name'],
 'resistance' => $data['resistance'],
 'wrapped' => $data['wrapped'],
 'category' => $data['category'],
 'description' => $data['description'],
 'wraps' => $data['wraps'],
 'images' => "coils/" . $uniqueID . "/",
 'wire_one' => $data['wire_one'],
 'wire_two' => $data['wire_two'],
 'wire_three' => $data['wire_three'],
 'wire_four' => $data['wire_four'],
 'wire_five' => $data['wire_five'],
 'wire_six' => $data['wire_six']
];
$db->insertFew("coils", array_keys($queryData), array_values($queryData));

This alone makes the method body much shorter.