MediCoder Aide

Blogues » MediCoder » Assignations compactes vs. complexité cyclomatique
Par mytto20854 points 

Assignations compactes vs. complexité cyclomatique

Voici un beau fragment de code regroupant trois séries d'assignations qui mériteraient d'être refactorées, dans la double optique d'être plus compact et lisible, afin d'en faire ressortir la logique plus clairement.

function updateFormFields() {
  parent::updateFormFields();
 
  $this->codes_ccam = strtoupper($this->codes_ccam);
  if ($this->codes_ccam) {
    $this->_codes_ccam = explode("|", $this->codes_ccam);
  } 
  else {
    $this->_codes_ccam = array();
  }
 
  $this->_hour_op = substr($this->temp_operation, 0, 2));
  $this->_min_op  = substr($this->temp_operation, 3, 2));
 
  if ($this->libelle_sejour) {
    $this->_view = $this->libelle_sejour;
  } 
  elseif ($this->libelle) {
    $this->_view = $this->libelle;
  } 
  else {
    $this->_view = $this->codes_ccam;
  }
}

Opérateur ternaire

Un cas typique d'opérateur ternaire manquant :

$this->codes_ccam = strtoupper($this->codes_ccam);
if ($this->codes_ccam) {
  $this->_codes_ccam = explode("|", $this->codes_ccam);
} 
else {
  $this->_codes_ccam = array();
}

Au delà de la syntaxe qui est plus compacte, cela permet de mettre en évidence que la condition n'a pour but que de faire varier l'affectation, et non de créer une branche cyclomatique nouvelle, avec tout un — possible — tas d'opérations impératives.

$this->_codes_ccam = this->codes_ccam ? 
  explode("|", strtoupper($this->codes_ccam)) : 
  array();

list + explode vs. substr

Ici on veut séparer les composantes HH et MM d'une variable de type ISO TIME HH:MM.

$this->_hour_op = substr($this->temp_operation, 0, 2));
$this->_min_op  = substr($this->temp_operation, 3, 2));

Si la syntaxe de substr() est très puissante et son exécution rapide, utiliser explode() permet de mieux mettre en évidence la séparation des composantes, tout en étant plus compact.

En outre, la syntaxe est beaucoup plus robuste si par malheur les composantes n'ont qu'un chiffre, ce qui arrive potentiellement lorsque la valeur ne vient pas de directement des données de Mediboard, par exemple 8:20 au lieu de 08:20. Ce comportement pad proof est celui de la plupart des systèmes compatibles avec ISO 8601 (external link), comme par exemple MySQL ou PHP

list($this->_hour_op, $this->_min_op) = explode(":", $this->temp_operation);

Valeurs par défaut en série

Enfin, ce dernier fragment montre un cas typique de valeurs par défaut en série, ou valeurs prioritaires, que l'on cherche pour la première d'entre elles qui n'évalue pas à false — cad null ou la chaine vide dans la plupart de cas.

if ($this->libelle_sejour) {
  $this->_view = $this->libelle_sejour;
} 
elseif ($this->libelle) {
  $this->_view = $this->libelle;
} 
else {
  $this->_view = $this->codes_ccam;
}

On aurait déjà pu compacter le code en utilisant un double opérateur ternaire, seulement la lisibilité n'est pas merveilleuse, surtout lorsqu'on a de nombreuses valeurs dans la série. Par ailleurs ce genre de logique est très fréquemment utilisée dans Mediboard, ainsi le framework fournit un opérateur qui permet de retourner la première valeur !false d'une série variatique.

$this->_view = CValue::first($this->libelle_sejour, $this->libelle, $this->codes_ccam);

Au total

Au total nous avons un code plus concis et plus clair donc plus maintenable.

function updateFormFields() {
  parent::updateFormFields();
 
  $this->_codes_ccam = this->codes_ccam ? 
    explode("|", strtoupper($this->codes_ccam)) : 
    array();
 
  list($this->_hour_op, $this->_min_op) = explode(":", $this->temp_operation);
 
  $this->_view = CValue::first($this->libelle_sejour, $this->libelle, $this->codes_ccam);
}

Nous avons vu également qu'il était légèrement plus robuste !

Sponsors privilégiés

Mediboard project