CODEWITHSUNDEEP
phpconditional-statementsvariable-assignment
user6791921
user6791921

Reputation:

Is this a good way of writing long conditions in PHP?

I have to evaluate a very long condition in PHP, so, to avoid errors and trying to write more readable code, I did the following:

 //this returns 1 when true, and nothing when false, although expected TRUE or FALSE

  $isNameValid=strlen($dataDecoded['nombre'])>=3;

  $isDescriptionValid=(strlen($dataDecoded['descripcion'])>=10) &&  strlen($dataDecoded['descripcion'])<=300;

  $isPriceValid=$dataDecoded['precio'] >0;

  $isImageValid=(($dataDecoded['imagen'] != "") && ($dataDecoded['imagen'] != NULL) );

And now, I can make the following:

 if($isNameValid==1 && $isDescriptionValid==1 && $isPriceValid==1 && $isImageValid==1)
  {
      echo "ok";
  }
  else{
      echo "no";
  }

It seems to work fine, but maybe is a weird way of doing things. I wanted to avoid the following, which I find more confusing and easy to make a mistake

if(strlen($dataDecoded['nombre'])>=3 && ... && ...)

Is there a better way to do that? Is wrong what I did? Thanks

Upvotes: 0

Views: 189

Answers (3)

ggorlen
ggorlen

Reputation: 57175

I don't care for creating extra variables here; this makes code difficult to maintain and unreusable. I'd recommend breaking your validation logic into easy-to-read, maintainable, reusable functions:

function valid($data) {
    return validName($data['nombre']) && 
           validDescription($data['descripcion']) &&
           validPrice($data['precio']) &&
           validImage($data['imagen']);
}

function validName($name) {
    return strlen($name) >= 3;
}

function validDescription($desc) {
    return strlen($desc) >= 10 && strlen($desc) <= 300;
}

function validPrice($price) {
    return $price > 0;
}

function validImage($image) {
    return $image !== "" && $image != NULL;
}

$dataDecoded = [
    "nombre" => "foo",
    "descripcion" => "foo bar foo bar",
    "precio" => 15,
    "imagen" => "foo.png"
];

// now your main code is beautiful:
echo (valid($dataDecoded) ? "ok" : "no") . "\n";

Upvotes: 2

abr
abr

Reputation: 2129

It really depends on how you want to handle it. Is switch an option or a viable one? Is ternary if prettier or handy?

From what I see, I'm guessing you have a validation purpose and a operating incoming depending on the validation. Why not create a function or a class that handles your input and validates? And in there, you can have all the dirty code you'd want. On your logical code, you'd just have to do (e.g of a class)

$someClass = new SomeClass();
$someClass->validate($fields);
if ($someClass->isValidated()) ...

This way, you'd actually follow some standards whereas the purpose of it would be to work as a validator for (all of? depends on your needs) your data

E.g of ternary ifs

$isNameValid = count($dataDecoded['nombre'])>=3 ? true : false;
$isDescriptionValid = count($dataDecoded['descripcion']) >= 10 && count($dataDecoded['descripcion']) <= 300 ? true : false;
$isPriceValid = count($dataDecoded['precio']) > 0 ? true : false;
$isImageValid = empty($dataDecoded['imagen']) === false ? true : false;


if ($isNameValid && $isDescriptionValid && $isPriceValid && $isImageValid) ...

Upvotes: 0

Liftoff
Liftoff

Reputation: 25392

Yes, that is acceptable. However, your variables there are all boolean, so you don't even need the ==1.

if($isNameValid && $isDescriptionValid && $isPriceValid && $isImageValid)

Upvotes: 2

Related Questions