Bug 10291

Summary: Misleading Variable names in batchMod.pl
Product: Koha Reporter: Colin Campbell <colin.campbell>
Component: Architecture, internals, and plumbingAssignee: Colin Campbell <colin.campbell>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, jonathan.druart, srdjan
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: proposed patch
Updated patch
[SIGNED-OFF] Bug 10291: Clarify misleading variable name
Bug 10291: Clarify misleading variable name

Description Colin Campbell 2013-05-20 15:54:16 UTC
batchMod,pl has two variables with names of the form x_hashref but both are actually references to an array of hashrefs. It passes the contents of one to the templares under the name itemnumbers_hashref but the use of it belies that name.

For the sake of future maintainers lets clean up these names
Comment 1 Colin Campbell 2013-05-20 16:03:57 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2013-06-03 04:38:00 UTC
It looks pointless to me to have hashrefs with only one key 'itemnumber'. Would you consider changing those arrays to be pure itemnumbers please, while you're at it?
Comment 3 Colin Campbell 2013-06-03 14:07:23 UTC
(In reply to comment #2)
> It looks pointless to me to have hashrefs with only one key 'itemnumber'.
> Would you consider changing those arrays to be pure itemnumbers please,
> while you're at it?

Yes in fact that simplifies the patch. Will post an update
Comment 4 Colin Campbell 2013-06-03 14:22:03 UTC Comment hidden (obsolete)
Comment 5 Srdjan Jankovic 2013-06-04 01:54:45 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-06-17 15:26:04 UTC
QA comment:

Variable renaming.
Need to modify the max amount of items (l. 113) and see the html code source for testing this patch.
No regression, no change detected.

Marked as Passed QA.
Comment 7 Jonathan Druart 2013-06-17 15:26:16 UTC
Created attachment 19076 [details] [review]
Bug 10291: Clarify misleading variable name

Remove the unrequired extra processing and pass
the array of itemnumbers as itemnumbers_array

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 8 Galen Charlton 2013-06-17 16:02:29 UTC
Pushed to master.  Thanks, Colin!