Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables

File /core/lib/Drupal/Core/Path/AliasManager.php

Line 107: Unused local variable $original_path
Line 264: Unused local variable $source

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

duozersk’s picture

Assigned: duozersk » Unassigned
Status: Active » Needs review
FileSize
705 bytes

Not sure if we can somehow fix the $source variable. The patch only fixes the $original_path.

AndyB

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Sure, $source needed here, because could not be defined as used in second part of condition

PS: phpstorm could be wrong, I've filed issue to their tracker WI-19665

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Nope phpstorm is not wrong $source is always defined again by the second part of the if statement as this will always be executed because the condition is an OR.

To be honest I think the code would be more readable if the was this

      $source = array_search($path, $this->lookupMap[$langcode]);
      if (!isset($this->lookupMap[$langcode]) || !$source) {
duozersk’s picture

Assigned: Unassigned » duozersk

Will do.

duozersk’s picture

Status: Needs work » Needs review
FileSize
1.3 KB

Status: Needs review » Needs work

The last submitted patch, drupal-remove-unused-var-2062245-5.patch, failed testing.

duozersk’s picture

Status: Needs work » Needs review
FileSize
1.3 KB

Yep, I should think more when following the advices ;)

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Much more readable

duozersk’s picture

Assigned: duozersk » Unassigned
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Great job!

Committed and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.