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.
Part of meta-issue #2002650: [meta, no patch] improve maintainability by removing unused local variables
File /core/modules/views/views.module
Line 125: Unused local variable $plugin
Line 196: Unused local variable $display_id
Line 1264: Unused local variable $field_name
Line 1272: Unused local variable $area_name
Line 1289: Unused local variable $field_name
Line 1297: Unused local variable $area_name
Comment | File | Size | Author |
---|---|---|---|
#6 | core-remove-unused-local-variables-views.module-2057157.patch | 2.77 KB | legolasbo |
Comments
Comment #1
legolasboI've located all six of these variables and have come to the conclusion that every one of them is a key in a foreach loop. I think removing these won't do the performance of their functions any good, but will reduce code understandability.
In my opinion we should leave these variables untouched.
Comment #2
tstoeckler@legolasbo: Can you explain what you mean? I think removing the key in the foreach() makes sense. I don't know how it reduces understandability. Can you explain? Thanks.
Comment #3
legolasboPerhaps it's just me, but I usually like to know what an array is keyed by if I loop trough it. Even though i don't always use the key variable I always define it and give it a meaningfull name. If I ever need to alter something I will instantly know what the array is keyed by without having to guess/look trough documentation/use devel.
Comment #4
tstoecklerHmm, that goes directly against #2002650: [meta, no patch] improve maintainability by removing unused local variables. I guess it really depends but in many cases I strongly disagree with you, such as:
I don't want to know if its
or
or whatever. I just want a
$thing
, do whatever I want with it and be done. Also not using variables can be risky for the reasons explained in the meta issue. If the code were to use the second example in this post, and my$thing
actually contains a weighted list of other things I can easily end-up re-using$weight
which will lead to bugs.So, yeah, I really think we should do this, actually.
Comment #5
legolasboI've also given this some thought over the past couple of days and i agree with your case in #4. The risk of introducing bugs outweighs the benefit of knowing what a key represents. I'll remove the variables and post a patch shortly.
Comment #6
legolasboAttached patch removes the unused variables
Comment #7
tstoecklerAwesome. I checked that the variables are in fact unused. Thanks!
Comment #8
webchickCommitted and pushed to 8.x. Thanks!