PHP return validate method of if one error is found

Question

My method counts the number of times an error occurs.

I have a feeling my snippet isn't the best practice way of doing this. So when one of the validation methods fails I want to stop the method.

public static function validate($data)
{
    $errors = [
        'settings_mismatch' => 0,
        'name_not_set' => 0,
        'price_not_set' => 0,
        'payment_qty_not_set' => 0,
        'payment_qty_zero' => 0,
        'payment_unit_not_set' => 0,
        'payment_unit_not_allowed' => 0
    ];


    if (!isset($data['name'])) {
        $errors['name_not_set']++;
        return $errors;
    }

    if (!isset($data['price'])) {
        $errors['price_not_set']++;
        return $errors;
    }

    if (!isset($data['payment_qty'])) {
        $errors['payment_qty_not_set']++;
        return $errors;
    }

    if (!isset($data['payment_unit'])) {
        $errors['payment_unit_not_set']++;
        return $errors;
    }

    if (isset($data['payment_qty']) && $data['payment_qty'] == 0) {
        $errors['payment_qty_zero']++;
        return $errors;
    }

    if (isset($data['payment_unit']) && !in_array(strtolower(self::transformUnit($data)['payment_unit']), self::$allowedMeasurements)) {
        $errors['payment_unit_not_allowed']++;
        return $errors;
    }

    return $errors;
}

Answer 1

If you are looking for a shorter code you can do this :

public static function validate($data)
{
    $errors = [
        'settings_mismatch' => 0,
        'name_not_set' => 0,
        'price_not_set' => 0,
        'payment_qty_not_set' => 0,
        'payment_qty_zero' => 0,
        'payment_unit_not_set' => 0,
        'payment_unit_not_allowed' => 0
    ];

    if (!isset($data['name']))
        return array_merge($errors,['name_not_set'=>1]);

    if (!isset($data['price']))
        return array_merge($errors,['price_not_set'=>1]);

    if (!isset($data['payment_qty']))
        return array_merge($errors,['payment_qty_not_set'=>1]);

    if (!isset($data['payment_unit']))
        return array_merge($errors,['payment_unit_not_set'=>1]);

    if (isset($data['payment_qty']) && $data['payment_qty'] == 0)
        return array_merge($errors,['payment_qty_zero'=>1]);

    if (isset($data['payment_unit']) && !in_array(strtolower(self::transformUnit($data)['payment_unit']), self::$allowedMeasurements))
        return array_merge($errors,['payment_unit_not_allowed'=>1]);

    return $errors;
}

But using function is always better, so for a cleaner code :

public static function validate($data)
{
    $methods = ['settings_mismatch','name_not_set','price_not_set','payment_qty_not_set','payment_unit_not_set','payment_qty_zero','payment_unit_not_allowed'];
    $errors = array();
    foreach($methods as $method)
        $errors[$method] = 0;
    foreach($methods as $method)
        if(self::$method($data))
        {
            $errors[$method] = 1;
            break;
        }
    return $errors;
}

public static function settings_mismatch(&$data)
{
    # TODO
}

public static function name_not_set(&$data)
{
    return !isset($data['name']);
}

public static function price_not_set(&$data)
{
    return !isset($data['price']);
}

public static function payment_qty_not_set(&$data)
{
    return !isset($data['payment_qty']);
}
public static function payment_qty_zero(&$data)
{
    return isset($data['payment_qty']) && $data['payment_qty'] == 0;
}

public static function payment_unit_not_set(&$data)
{
    return !isset($data['payment_unit']);
}
public static function payment_unit_not_allowed(&$data)
{
    return isset($data['payment_unit']) && !in_array(strtolower(self::transformUnit($data)['payment_unit']), self::$allowedMeasurements);
}

Then I wonder why would you break on the first error instead of returning all errors (a better practice if you are implementing a Web service) :

public static function validate($data)
{
    $methods = ['settings_mismatch','name_not_set','price_not_set','payment_qty_not_set','payment_unit_not_set','payment_qty_zero','payment_unit_not_allowed'];
    $errors = array();
    foreach($methods as $method)
        $errors[$method] = (int)self::$method($data);
    return $errors;
}
# same other methods as above

Please note that in the 2 last codes the order of $methods is not the same as the function order, to keep your original code behavior. In the last code since the order is not relevant, you could reflect the class to get the list of $methods.

Also as a code review there is a wrong logic (too me) : instead of isset && ==0 or isset && !in_array I would have put !isset || ==0 and !isset || !in_array, to imply that a not set quantity is as not allowed as a zero quantity, and that a not set unit is not allowed too, but you can keep it this way there is no error here.