DEV Community

Michael Lueckl
Michael Lueckl

Posted on • Updated on

Best Practice - Check if property exist and assign

Hi all,

from time to time you might need to assign properties or similar to variables and in some cases some don't exist.

In PHP and most other languages it's pretty straightforward to check for example:

~~~~ php
if(isset($issue['samples']))
$i->samples = $issue['samples'];
~~~~

This is also ok for multiple cases:

~~~~ php
if(isset($issue['samples']))
$i->samples = $issue['samples'];

if(isset($issue['summary']))
$i->summary = $issue['summary'];

if(isset($issue['property_names']))
$i->property_names = $issue['property_names'];

if(isset($issue['description']))
$i->description = $issue['description'];

if(isset($issue['count']))
$i->count = $issue['count'];
~~~~

And at some point I came up with inline if's which makes it easier to write/read and maintain.

~~~~ php
$i->samples = (isset($issue['samples']) ? $issue['samples'] : null);
$i->summary = (isset($issue['summary']) ? $issue['summary'] : null);
$i->property_names = (isset($issue['property_names']) ? $issue['property_names'] : null);
$i->description = (isset($issue['description']) ? $issue['description'] : null);
$i->count = (isset($issue['count']) ? $issue['count'] : null);
~~~~

But I feel there might be other and better way's of handling this.

Here the full example with a foreach-loop:

~~~~ php
foreach($data['quality_issues']['data'] as $issue){
if($issue['property_names'] != 'product_category'){
print_r($issue);
$i = new QualityIssue();
$i->samples = (isset($issue['samples']) ? $issue['samples'] : null);
$i->summary = (isset($issue['summary']) ? $issue['summary'] : null);
$i->property_names = (isset($issue['property_names']) ? $issue['property_names'] : null);
$i->description = (isset($issue['description']) ? $issue['description'] : null);
$i->count = (isset($issue['count']) ? $issue['count'] : null);
$i->error = (isset($issue['error']) ? $issue['error'] : null);
$i->name = (isset($issue['name']) ? $issue['name'] : null);
$i->type = (isset($issue['type']) ? $issue['type'] : null);
$i->level = (isset($issue['level']) ? $issue['level'] : null);
$i->category = (isset($issue['category']) ? $issue['category'] : null);

    array_push($partner->catalog->quality_issues, $i);
}
Enter fullscreen mode Exit fullscreen mode

}
~~~~

Maybe I'm overthinking this and the inline if is already the "perfect" solution (as if this exists :)) but I would like to hear from you how you normally handle this and if you came up with good practices that might help to improve this.

Cheers,
Michael

CONCLUSION:

Really appreciate the great feedback in the comments and wanted to describe the solution which I found most suitable suggested by @franco Traversaro:

~~~~ php
class QualityIssue{
public static function fromArray(array $source) : self
{
$i = new self();
foreach ($source as $property => $value) {
$i->{$property} = $value;
}

    return $i;
}
Enter fullscreen mode Exit fullscreen mode

}

foreach($data['quality_issues']['data'] as $issue){
if($issue['property_names'] != 'product_category'){
array_push($partner->catalog->quality_issues, QualityIssue::fromArray($issue));
}
}
~~~~

Top comments (13)

Collapse
 
moopet profile image
Ben Sinclair

One way to make this shorter is to use PHP7's null coalesce operator:

$i->samples = $issues['samples'] ?? null;

and to make it a bit DRYer, you could loop through the properties:

$properties = ['samples', 'summary', 'property_names', ]; // etc.

foreach ($properties as $property) {
    $i->{$property} = $issues[$property] ?? null;
}

If you know that the $properties array only contains elements which are suitable for pushing onto the $i object you could go with Alex Cioroianu's suggestion.

Collapse
 
belinde profile image
Franco Traversaro

Personally I would move that logic inside a QualityIssue static method:

class QualityIssue {
   public static function fromArray(array $source): self
   {
      $properties = ['samples', 'summary', 'property_names', ]; // etc.
      $instance = new self();
      foreach ($properties as $property) {
         $instance->{$property} = $issues[$property] ?? null;
      }

      return $instance;
   }
}

In this way the acceptable fields are listed only inside the class that define them, and the properties could also be protected or private (* see below). And the code of the OP could simply be:

foreach($data['quality_issues']['data'] as $issue){
    if($issue['property_names'] != 'product_category'){
        $partner->catalog->quality_issues[] = QualityIssue::fromArray($issue);
    }
}

(*) because yes, you can access a private property from outside the object, if you do it from a method of that class:

class Foo {
   private $myProp = 'dog';

   public static function changeProp(Foo $foreign): void
   {
      $foreign->myProp = 'cat';
   }
}

$subject = new Foo();
Foo::changeProp($subject);
Collapse
 
mlueckl profile image
Michael Lueckl • Edited

I do really like this approach!
I wasn't aware you can assign property names like this and creating a static class is so much cleaner and provides a lot of benefits.

This is what I was looking for :)

Can you please explain what
: self
exactly is used for?
From what I understand it allows you to create an instance of the class inside the static method but it works also when I remove it.
I don't really know how to search for it and couldn't find an explanation.

Cheers

Thread Thread
 
belinde profile image
Franco Traversaro

It's a PHP 7 feature, it's called "return type declaration": basically you declare that the function will always return something having the declared type; if you return something else, like an integer, or don't return anything, a TypeError is thrown. I really like this feature, strict type force you to write better code.

Thread Thread
 
mlueckl profile image
Michael Lueckl • Edited

This is great, guess most of the time I use PHP in a too "simple" way and should get more familiar with such details as it really helps to improve and ease the code :)

It's basically like in C++, or similar languages, when you define a function with a specific return type like

static QualityIssue fromArray(vector<type> &source){}

Thanks for the explanation, I struggled to find a good search query for this question with only the code.

Collapse
 
mlueckl profile image
Michael Lueckl

PHP7's null coalesce operator is awesome, thank you for pointing this out.
I like the other options more for the purpose of my question but this is a great "upgrade" from the inline if.

I do like Alex Cioroianu suggestion as I don't really need to assign a value if it does not exist but this is still helpful if I need to assign some fallback value or similar.

Cheers

Collapse
 
alexcioroianu profile image
Alex Cioroianu

why not do an foreach over the issues?
foreach($issue as $key=>$value){$i->{$key}=$value;}

sorry for missing spaces, but i am writing from my phone.

Also on if statement, even if there is one action, use the brackets!

Collapse
 
mlueckl profile image
Michael Lueckl

That's awesome! I wasn't aware you can set the property name like this which makes my case so much easier :D

Can you please elaborate on why if statement should always use brackets?
So far I did not run into problems here but would like to understand your reason for saying this.

Cheers

Collapse
 
belinde profile image
Franco Traversaro

Consider following PSR-1 and PSR-2 coding standards. PSR are recommendations of the php-fig group, they're becoming a de facto standard. The more developers follow a single style standard, the easier become understand someone else code.

Thread Thread
 
mlueckl profile image
Michael Lueckl

This is great and I will look into this.
I always try to adjust my style to a standard because I strongly agree that standards are very useful.

Collapse
 
alexcioroianu profile image
Alex Cioroianu

i always use brackets because it is easyer to read and extend.

for example, if you want to add another action inside the if statement, you need to add the brackets anyway.

it’s a best practice that will help you and the developes after you to better understand the logic.

Thread Thread
 
mlueckl profile image
Michael Lueckl

I did indeed run into the case that I added another statement and missed the brackets :)
But overall I agree, was thinking there is a specific technical reason.

Thanks!

Collapse
 
willvincent profile image
Will Vincent

And at some point I came up with inline if's which makes it easier to write/read and maintain.

AKA Ternary statements.

I point this out because it's helpful to know and use the name, and googling "? :" doesn't work out too well. I remember trying to do so years ago when I wanted to know wtf these were called so that I could really understand what all I could do with them. :)

Also, while they may be less to write, it could be argued (successfully I think) that ternaries are more difficult to read than if/elseif/else statements with brackets. Though for small things, as in your example here, they probably do make more sense than an if/else block for every property.

If you find yourself doing things like this though, almost definitely reach for the if/elseif/else blocks because while valid, this gets ugly fast...

for ($i = 1; $i <= 100; $i++) {
    $mod3 = $i % 3;
    $mod5 = $i % 5;
    echo (!$mod3 && !$mod5 ? "FizzBuzz" : (!$mod3 ? "Fizz" : (!$mod5 ? "Buzz" : $i))) ."\n";
}

:D