[Bug 5453] New: Variables declaration should not occur in conditionals
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Summary: Variables declaration should not occur in conditionals Change sponsored?: --- Product: Koha Version: HEAD Platform: All OS/Version: All Status: NEW Severity: enhancement Priority: P5 Component: Architecture, internals, and plumbing AssignedTo: gmcharlt@gmail.com ReportedBy: colin.campbell@ptfs-europe.com QAContact: koha-bugs@lists.koha-community.org Estimated Hours: 0.0 This is an antipattern in a lot of Koha code. my $x = Z() if $y; it obscures code and can cause hard to track bugs. What does it say of the value of $x if not $y? Is someone maintaining the code liable to make unwarrantable assumptions about the definedness or value of $x. ( the use of postfix if is also a problem as in reading actual code it is often easy to miss that an assignment is in a conditional because the postfix if has disappeared off the right of your screen. Postfix ifs give a reader very little visual clue that a conditional is occurring. The above cde should be rewritten as my $x; if ($y) { $x = Z(); } Bug opened to record such patches against -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Nighswonger <cnighswonger@foundations.edu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cnighswonger@foundations.ed | |u --- Comment #1 from Chris Nighswonger <cnighswonger@foundations.edu> 2010-11-29 20:36:05 UTC --- I realize Conway says that postfixed if's are not good style, but I think that as this is a coding "style" issue, it should at least be given an airing on the devel list prior to being implemented. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P5 |PATCH-Sent CC| |chris@bigballofwax.co.nz Patch Status|--- |Needs Signoff -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #2 from Colin Campbell <colin.campbell@ptfs-europe.com> 2010-12-02 11:18:54 UTC --- The issue isn't the postfix if per se but the declaration of the variable within a the conditional. It can cause run time warnings when the undefined undeclined variable is used later in the code -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #3 from MJ Ray (software.coop) <mjr@ttllp.co.uk> 2010-12-08 10:16:15 UTC --- This bug is mentioned in: Bug 5453 Do not declare variables in conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013116.... Bug 5453: Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013120.... Bug 5453: Move declarations from within conditionals in catalogue/search.pl http://lists.koha-community.org/pipermail/koha-patches/2010-November/013121.... Bug 5453 Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013122.... Bug 5453 Move declarations out of conditionals in opac http://lists.koha-community.org/pipermail/koha-patches/2010-November/013123.... Bug 5453 Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013124.... Bug 5453 : Move declarations out of conditionals http://lists.koha-community.org/pipermail/koha-patches/2010-November/013125.... -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #4 from M. de Rooy <m.de.rooy@rijksmuseum.nl> 2011-05-28 12:44:20 UTC --- Could you attach the proposed patches? -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #5 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:51:56 UTC --- Created attachment 4340 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4340 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #6 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:52:33 UTC --- Created attachment 4341 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4341 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #7 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:52:59 UTC --- Created attachment 4342 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4342 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #8 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:53:40 UTC --- Created attachment 4343 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4343 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #9 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:55:08 UTC --- Created attachment 4344 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4344 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #10 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:55:40 UTC --- Created attachment 4345 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4345 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #11 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 14:56:23 UTC --- Created attachment 4346 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4346 Proposed Patch -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 --- Comment #12 from Colin Campbell <colin.campbell@ptfs-europe.com> 2011-06-02 15:01:52 UTC --- Patches rebased against current HEAD and attached or they can be pulled from the bug_5453 branch on git://github.com/colinsc/koha.git -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4340|0 |1 is obsolete| | --- Comment #13 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 20:56:28 UTC --- Created attachment 4455 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4455 Bug 5453 Do not declare variables in conditionals Removed occurences in acqui/*.pl Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4341|0 |1 is obsolete| | --- Comment #14 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 20:57:46 UTC --- Created attachment 4456 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4456 Bug 5453: Move declarations out of conditionals Patch admin/*.pl scripts Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4342|0 |1 is obsolete| | --- Comment #15 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 21:03:41 UTC --- Created attachment 4457 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4457 Bug 5453: Move declarations from within conditionals in catalogue/search.pl Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4343|0 |1 is obsolete| | --- Comment #16 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 21:05:41 UTC --- Created attachment 4458 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4458 Bug 5453 Move declarations out of conditionals Fix im members scripts Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4344|0 |1 is obsolete| | --- Comment #17 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 21:09:31 UTC --- Created attachment 4459 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4459 Bug 5453 Move declarations out of conditionals in opac Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4345|0 |1 is obsolete| | --- Comment #18 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 21:11:19 UTC --- Created attachment 4460 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4460 Bug 5453 Move declarations out of conditionals Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #4346|0 |1 is obsolete| | --- Comment #19 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 21:14:19 UTC --- Created attachment 4461 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=4461 Bug 5453 : Move declarations out of conditionals Patched for C4 Modules Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Needs Signoff |Signed Off --- Comment #20 from Chris Cormack <chris@bigballofwax.co.nz> 2011-06-14 21:15:48 UTC --- Thanks for doing this Colin, it quiets the logs a lot! -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Ian Walls <ian.walls@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ian.walls@bywatersolutions. | |com --- Comment #21 from Ian Walls <ian.walls@bywatersolutions.com> 2011-07-14 22:29:34 UTC --- 1st attachment: acqui/*.pl: Passed QA 2nd attachment: admin/*.pl: Passed QA 3rd attachment: catalogue/search.pl: Passed QA 4th attachment: members/*.pl: Passed QA 5th attachment: opac/*.pl: Passed QA 6th attachment: lots of others: Passed QA 7th attachment: will require more time to read through, as the changes seem more complex, and will affect more of the system. Please stand by. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Ian Walls <ian.walls@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Signed Off |Passed QA --- Comment #22 from Ian Walls <ian.walls@bywatersolutions.com> 2011-07-15 02:49:05 UTC --- 7th attachment: C4 modules: Passed QA. Marking bug report as such. -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=5453 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch Status|Passed QA |Patch Pushed --- Comment #23 from Chris Cormack <chris@bigballofwax.co.nz> 2011-07-15 03:50:50 UTC --- Pushed, please test -- Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA Contact for the bug.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org