CODEWITHSUNDEEP
php
Ruben Bårdsen
Ruben Bårdsen

Reputation: 15

Avoiding messy validation code

I am currently making a simple login/register system for my website, and I'm not so happy about a part of my code.

I have a script that checks if any of the fields in the register-form is empty, and gives a proper response, say if you forgot to type a first name, it tells you "First name cannot be empty".

My problem is that my code currently looks like this: 

if(field1 != ""){
  if(field2 != ""){
    if(field3 !=""){
      //register user
    }else{
      //field3 empty error
    }
  }else{
  //field2 empty error
  }
}else{
//field1 empty error
}

Is there a cleaner and more efficent way of doing this, that still lets me show exactly what field is left empty?

Upvotes: 0

Views: 145

Answers (6)

svgrafov
svgrafov

Reputation: 2014

You can use foreach loop with array of fields and validation messages. Just put your form fields in another array($form)(or use variable variables).

$fields = [
'field1' => 'empty field1',
'field2' => 'empty field2',
'field3' => 'empty field3'
];
$errors = [];

foreach ($fields as $field => $message){
  if ($form[$field] !== ''){
    $errors[$field] => $message;
  }
}

if ($errors){
  //report errors
}

Bonus: you can add different validation rules to your $fields array. I will use beberlei/assert library, but you can pick anything you want.

use Assert\Assertion;
$fields = [
  'field1' => ['message' => 'empty field1', 'rule' => 'notEmpty'
];

$errors = [];

foreach ($fields as $field => $rules){
   try {
     call_user_func_array('Assertion::' . $rules['rule'], [$form[$field], $rules['message']]);
   } catch(AssertionFailedException $e) {
     $e->getValue(); // the value that caused the failure
   }
}

Upvotes: 1

Tharindu Thisarasinghe
Tharindu Thisarasinghe

Reputation: 3998

It's always good to use validation functions on a separate php file and include them on whatever the page you want.

For what you're doing, the presence check, you can use a simple foreach loop to check all the necessary fields if they're empty.

On your register.php page :

$required_fields = array('first_name','last_name', 'phone', 'email', 'password');
validate_has_presense($required_fields);

On validation_functions.php page

function has_presense($value){
    return isset($value)&& $value !== "";
}

function validate_has_presense($required_fields){
    global $errors;

    foreach ($required_fields as $field){
        $value = trim($_POST[$field]);
        if(!has_presense($value)){
            $errors[$field] = "Please enter ".str_replace("_", " ", $field);
        }
    }
}

Using this way of presence validation is much more cleaner than writing if and elseif over and over again...

Upvotes: 0

Bojan Radaković
Bojan Radaković

Reputation: 440

Maybe you'd want to check each of the fields by order, and return only one error, if it occurs.

function error($message) {
    exit('Error: ' . $message);
}

if ($field1 == "") { //You can also use if(empty(trim($field1)))
    error('Empty field 1');
}
if ($field2 == "") {
    error('Empty field 2');
}
if ($field3 == "") {
    error('Empty field 3');
}
... //Do your magic

Instead of exit() in the error function, you can use your own logic, depending on the code. Hope this helps

Upvotes: 0

Martijn
Martijn

Reputation: 16103

You can use elseif, and reverse the logic to first test if everything is neat:

    if( $field1 =="" ){ $error = "no field1 value"; }
elseif( $field2 =="" ){ $error = "no field2 value"; }
elseif( $field3 =="" ){ $error = "no field3 value"; }
else{
    // your magic here, everything is clean.
}

Alternatively you can use an array and remember all errors in one go:

$errors = [];

if( $field1 =="" ){ $errors[] = "no field1 value"; }
if( $field2 =="" ){ $errors[] = "no field2 value"; }
if( $field3 =="" ){ $errors[] = "no field3 value"; }

// Again, first check if we have any problems:
if( count($errors)!==0 ){
    echo "something was incorrect:<br />".implode("<br />", $errors);
}
else{
    // your magic here, everything is clean.
}

Upvotes: 0

SoftGuide
SoftGuide

Reputation: 336

What about style like this:

$aErrors = array();

if(field1 == "") {
    $aErrors[] = 'error 1';
}
if(field2 == "") {
    $aErrors[] = 'error 2';
}
if(field3 == "") {
    $aErrors[] = 'error 3';
}

if(empty($aErrors)) register();
else print_errors();

Upvotes: 0

cn0047
cn0047

Reputation: 17071

You may try something like this:

<?php

$errors = [];
if ($field1 === '') {
  $errors['field1'] = 'Empty error.'.
}
if ($field2 === '') {
  $errors['field2'] = 'Empty error.'.
}
if ($field3 === '') {
  $errors['field3'] = 'Empty error.'.
}

if (count($errors) === 0) {
  // Registration
} else {
  // Error
}

Upvotes: 2

Related Questions