Warning d'inspection dans une fonction #4424

Open
opened 4 months ago by Eric · 2 comments
Eric commented 4 months ago
Owner

Dans la méthode suivante, la variable $retour n'est pas utilisée.
Soit elle est inutile car on se fout du retour, soit il manque un traitement conditionnel au retour qui influerait sur la sortie de la fonction retirer().

Si quelqu'un a une idée pour qu'on corrige cela et si possible qu'on évite des return partout si le $retour est bien la variable de retour de la fonction retirer().

/**
 * {@inheritDoc}
 */
public function retirer(array $fichiers = []): bool {
	$archive = $this->archiveEnEcriture();
	if ($archive) {
		if (1 === $archive->open($this->fichier_archive, 'edition')) {
			// Vérifier qu'on ne cherche pas à vider l'archive
			$reste = $this->informer();
			$fichiers_restants = array_column($reste['fichiers'], 'filename');
			if (0 === count(array_diff($fichiers_restants, $fichiers))) {
				$this->setErreur(8);
					return false;
			}
			$retour = $archive->remove($fichiers);
			$archive->close();
		}
		$this->setErreur(0);
			return true;
	}
	return false;
}

Dans la méthode suivante, la variable $retour n'est pas utilisée. Soit elle est inutile car on se fout du retour, soit il manque un traitement conditionnel au retour qui influerait sur la sortie de la fonction retirer(). Si quelqu'un a une idée pour qu'on corrige cela et si possible qu'on évite des return partout si le $retour est bien la variable de retour de la fonction retirer(). ```php /** * {@inheritDoc} */ public function retirer(array $fichiers = []): bool { $archive = $this->archiveEnEcriture(); if ($archive) { if (1 === $archive->open($this->fichier_archive, 'edition')) { // Vérifier qu'on ne cherche pas à vider l'archive $reste = $this->informer(); $fichiers_restants = array_column($reste['fichiers'], 'filename'); if (0 === count(array_diff($fichiers_restants, $fichiers))) { $this->setErreur(8); return false; } $retour = $archive->remove($fichiers); $archive->close(); } $this->setErreur(0); return true; } return false; } ```
Owner

$archive->remove() retourne un booleen

a priori ça serait donc

		$this->setErreur(0);
		return $retour;
`$archive->remove()` retourne un booleen a priori ça serait donc ```php $this->setErreur(0); return $retour; ```
Owner

Ah non…

	if ($archive) {
		if (1 === $archive->open($this->fichier_archive, 'edition')) {
			// Vérifier qu'on ne cherche pas à vider l'archive
			$reste = $this->informer();
			$fichiers_restants = array_column($reste['fichiers'], 'filename');
			if (0 === count(array_diff($fichiers_restants, $fichiers))) {
				$this->setErreur(8);
					return false;
			}
			$retour = $archive->remove($fichiers);
			$archive->close();
            $this->setErreur(0);
			return $retour;
		}
	}

peut être plutôt, sinon $retour pourrait ne pas être défini après le if.

Ah non… ```php if ($archive) { if (1 === $archive->open($this->fichier_archive, 'edition')) { // Vérifier qu'on ne cherche pas à vider l'archive $reste = $this->informer(); $fichiers_restants = array_column($reste['fichiers'], 'filename'); if (0 === count(array_diff($fichiers_restants, $fichiers))) { $this->setErreur(8); return false; } $retour = $archive->remove($fichiers); $archive->close(); $this->setErreur(0); return $retour; } } ``` peut être plutôt, sinon `$retour` pourrait ne pas être défini après le if.
b_b added the
amélioration
label 3 months ago
b_b added this to the spip-4.2 milestone 3 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.