Exakat 1.5.3 Review

Exakat 1.5.4 add more after-coding review for your code : after suggesting upgrading visibility on properties, it now checks class candidates for final and abstract keyword. As you can guess, a class won’t be a candidate for them at the same time. We also include more review of generators, including the PHP classic trap with iterator_to_array(), and code improvement with yield from. I love it when the Exakat 1.5.3 review comes together.

Use Yield From

Yield from was introduced in PHP 7. It is a delegation keyword for yield : the yielding will be handed to another function or method, and when this delegated yield has been finished, the next will resume.

This means that yield from is basically a foreach loop for a designated generator, and may be used just like that :

  
<?php

function bar_good() {
    yield 1;
   yield from foo();
    yield 5;

}

function bar_bad() {
    yield 1;
    foreach(foo() as $f) {
        yield $f;
   }
    yield 5;

}
// For context
function foo() {
    yield 2;
    yield 3;
    yield 4;
}

?>

Note that yield from works with any Traversable object and arrays, on top of of other Generator. This means that the following are basically the same :

  
<?php

function bar_bad() {
   foreach(array('a', 'b', 'd', 'e', 'f') as $i) {
      yield $i;
   }
}

function bar_good() {
    yield from array('a', 'b', 'd', 'e', 'f');
}
?>

It is probably not worth turning a foreach on array() into a generator, but if the code is juggling with several sources of data, it is very convenient to build such a generator.

Using yield from not only hides the foreach loop, but it is also faster to execute.

Beware of iteratortoarray()

The yield keyword was introduced in PHP 5.5, to build generators. Those are used in foreach loops, and produce one value each time. To build an array from a generator, there is the convenient function iterator_to_array(), which collects all the values into one array, saving the call of a foreach().

  
<?php
function from() {
    yield 1; // key 0
    yield 2; // key 1
    yield 3; // key 2
}
function gen() {
    yield 0; // key 0
    yield from from(); // keys 0-2
    yield 4; // key 1
}
// returns [1, 4, 3]
var_dump(iterator_to_array(gen()));
?>

As the PHP manual says, the yield keyword provides automatic keys but doesn’t reset those when calling yield from. As such, mixing yield and yield from in the same generator is bound to lead to overwrite values in the final array.

Solutions to this problem include : using array_merge to merge the arrays after using iteratortoarray; using keys when yielding.

  
<?php
function from() {
    yield 1 => 1; // key 1
    yield 2 => 2; // key 2
    yield 3 => 3; // key 3
}
function gen() {
    yield 0; // key 0, automatic
    yield from from(); // keys 1-3
    yield 4 => 4; // key 4
}
// returns [0 1, 2, 3, 4]
var_dump(iterator_to_array(gen()));
?>

In this example, we simply reuse the value as key, and end up with distinct keys. If real code, when generators are arbitrarily combined, merging the arrays after the call to iteratortoarray() is a better idea.

Thanks to Holger Woltersdorf for running into it

Final or abstract after the code

final and abstract are two keywords that apply to classes and methods. They signal classes that can’t or must be extended, respectively. A final class can’t be found as extends, and abstract classes must be extends.

They are most often set at conception time, when the architect of the code has a broad view over it, and has freedom to add restrictions inside the code. When code conception is lightly enforced, or if architecture is grown organically, the code will be written without those keywords.

How can we check if those keywords are applicable ? Since the code is already written, we simply have to reverse the logic of the rule : a final class is a class that is never extended, but instantiated; an abstract class is a class that is extended, but never instantiated directly.

  
<?php

// Instantiated but never extended class
class couldBeFinal {      }
new couldBeFinal(); 

// Extended but never instantiated class
class couldBeAbstract {      }
class couldNotBeAbstract extends couldBeAbstract {      }

?>

Please, be gentle with this example : it is small, for editing purposes. On the other hand, reviewing a 2000 class-strong application to check which are extended and which are instantiated is usually a daunting task : as such, it is often delayed until never done.

All this is actually a good task for static analysis. Exakat collects all those operations across the entire repository, and report both situation in the Ambassador report (the default report). Here, you’ll find every class that may be upgraded to final or abstract.

The red stars mention possible upgrades by using final or abstract keywords; The green stars mention a valid absence of the option (an extended class, that can’t be final, for example); The absence of star reports currently configured classes : good job.

Is it enough to decide if a class may be final or abstract : probably not. This report is a great base to review the current class configuration, and ponder about the adding of such options. It is also interesting to review the list, and find classes that ought to be listed, but didn’t make it : maybe those abstractClass that were supposed to be extended did find some other usage. In any case, time for a code review!

The Weekly Audits : 2018, Week #46

Exakat includes a ‘weekly’ report : this report is built with a selection of five analyses. This means a short audit report, with few issues to review. This is not a lot to read them, and review them in your code. Everyone in the PHP community can focus on one of the classic coding problems and fix it. Talk about the weekly audit around you : you’ll find programmers facing the same challenges.

To obtain the ‘weekly’ audit, run an audit, and request the ‘Weekly’ report.

# Init the project (skip when it is already done)  php exakat.phar init -p <yourproject> -R https://github.com/Seldaek/monolog.git -git 

# Run the project (skip when it is already done)  php exakat.phar project -p <yourproject> 

# Export the weekly project (every monday)  php exakat.phar report -p <yourproject> -format Weekly 

# Open projects/<yourproject>/weekly/index.html in your browser

Every week, you can find here 5 new analysis to review in your code. In fact, when your code is clean, you can also take a quick look at the upcoming

Weekly recommendations for PHP code review : 2018, week 2018-46

Undefined static:: Or self:: : List of all undefined static and self properties and methods.
Useless Referenced Argument : The argument has a reference, but is only used for reading.
preg_replace With Option e : preg_replace() supported the /e option until PHP 7.
Must Call Parent Constructor : Some PHP native classes require a call to parent::__construct() to be stable.
Used Once Property : Property used once in their defining class.

Every week, you can find here 5 new analysis to review in your code. In fact, when your code is clean, you can also take a quick look at the upcoming week with the ‘Go further’ section.

Happy PHP Code Reviews

All the 362 analyzers are presented in the docs, including the faithful Switch Without Default : Always use a default statement in switch().. It is a routine bug : 56% applications mishandle switch().

You can check all of the Exakat reports at the gallery: exakat gallery.

Download Exakat on exakat.io, install it with Docker, upgrade it with ‘exakat.phar upgrade -u’ and like us on github.

Want to Keep in touch with us, subscribe to our newsletter !

Company

Exakat 1.5.3 review