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";
    } 

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.