Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.ListDataDefinition::getClass() contains the following code.
$class = isset($this->definition['class']) ? $this->definition['class'] : NULL;
if (!empty($class)) {
return $class;
}
It is a rather convoluted version of the following code.
if (isset($this->definition['class'])) {
return $this->definition['class'];
}
That is exactly the code used from DataDefinition::getClass().
if (isset($this->definition['class'])) {
return $this->definition['class'];
}
Also there isn't a need to use an if() {} else {} statement, since the if() part returns to the caller, when executed.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | simplify-code-2934096-4.patch | 1.59 KB | apaderno |
| #2 | simplify-code-2934096-2.patch | 1.59 KB | apaderno |
Comments
Comment #2
apadernoComment #3
joelpittetYou're right it does look convoluted but it should be
if (!empty($this->definition['class'])) {no? The!empty()does more than theisset()and an implicit isset.Comment #4
apadernoI assumed there is code that verifies the class name is not set to an empty string or
FALSEbecause the other method usesisset().I didn't check such code exists in Drupal, but I agree that at the point
getClass()is called,$this->definition['class']should already contain a valid class name, or not be set.If, eventually, an empty string does have a particular meaning, it should be converted to a real class name before
getClass()is called, or insidegetClass().Since at the moment there isn't such need, this is the updated patch.
Comment #5
joelpittetThat looks better, thanks for updating the patch.
Comment #7
apadernoComment #8
joelpittetThat's strange it says it passed... testbot! 👿
Comment #9
apaderno@joelpittet It was not passing because an error on DrupalCI (Requeued after CI error. Requeued after CI error. Requeued after CI error. Requeued after CI error. Requeued after CI error.) not because the patch is wrong.
Comment #10
larowlanCrediting @joelpittet as his review changed the shape of the patch
Comment #11
larowlanCommitted as 6eafe72 and pushed to 8.5.x.
Thanks
Comment #14
apaderno