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

}
~~~~

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

}

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

Comments

[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.

[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);

[Michael Lueckl]

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

[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.

[Michael Lueckl]

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.

[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

[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!

[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

[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.

[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.

[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.

[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!

[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