Beefy Boxes and Bandwidth Generously Provided by pair Networks
The stupid question is the question not asked
 
PerlMonks  

Re: foreach argument modification inside loop, followed by return from loop

by sundialsvc4 (Abbot)
on Jul 10, 2013 at 11:49 UTC ( #1043452=note: print w/ replies, xml ) Need Help??


in reply to foreach argument modification inside loop, followed by return from loop

If I were to look at that piece of code, without comments, then I would finger it as almost-certainly “a bug,” because I can’t imagine why you would want to pop the last element off of an array if any element in it equals '2.'   I am going to presume, frankly, that you (whoever you were) didn’t write what you thought you wrote, and that you never exhaustively tested it.   Now, I’ve got to figure out what your code actually does, what you meant to write, and what else might be wrong ... nearby or far-away to this questionable subroutine.   (Where there’s one roach in the kitchen, there are many.)

These admonitions about what to do and what not to do with arrays and lists are really (IMHO) just as much having to do with clarity (and future-proofing as maintenance happens), as with what will or won’t cause the interpreter to behave unexpectedly.   When you are tasked with maintaining hundreds of thousands of lines of years-old smelly code that you didn’t write, “cleverness” is not your friend.

(Please note that my uses of the word “you” are entirely impersonal ... “you” could be anyone, anywhere, anytime.)


Comment on Re: foreach argument modification inside loop, followed by return from loop
Re^2: foreach argument modification inside loop, followed by return from loop
by vsespb (Hermit) on Jul 10, 2013 at 12:09 UTC
    because I canít imagine why you would want to pop the last element off of an array if any element in it equals '2.
    Don't try to analyze code logic. It's a proof-of-concept code. Perl does not care about it's logic when execute it, so you should not too.
    clarity

    So, if don't wish to see proof-of-concept code without any sane logic, perhaps you want to see real code, here it is.
    do_finish() removes element from @{$self->{jobs_a}}, after that we exit "for" loop in get_task().
    Do you think it will be more clear if it will return number of element to remove, and we remove it outside for loop?
      Do you think it will be more clear if it will return number of element to remove, and we remove it outside for loop?

      Yes, IMHO. My approach, using my own conventions, would be something like this (untested):

      sub do_finish { my $self = shift; my ($jobid_to_delete) = @_; delete $self->{jobs_h}->{$jobid_to_delete}; my $i_to_delete; my $ar_jobs_a = $self->{jobs_a}; JID: for my $i (0 .. $#$ar_jobs_a) { my $jid = $ar_jobs_a->[$i]{jobid}; next JID unless $jid == $jobid_to_delete; $i_to_delete = $i; last JID; } splice @$ar_jobs_a, $i_to_delete, 1 if defined $i_to_delete; return @$ar_jobs_a ? 'ok' : 'done'; }

      Update: Renamed  $jobid to  $jobid_to_delete in interests of self-documentation.

        Yes, perhaps.

      vsespb, I sense that you have taken-offense at my comment, and I therefore wish to hasten to say that I meant nothing “personal” by it.   You see, through most of my career, I have dealt with “old code.”   None of the programmers who wrote the original code are still there ... they have moved on, and the legacy systems (of course) have not.   Therefore I am constantly (and quite impersonally ... every author is presumed to be a Consummate Pro) “looking for funny smells.”

      I don’t mean anything negative in my description of the sort of things that I have learned to look-for in source code.   Have I, or has anyone on this planet, written code that smells every bit as badly?   Alas, I have but one answer.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://1043452]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others making s'mores by the fire in the courtyard of the Monastery: (16)
As of 2014-09-30 17:19 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    How do you remember the number of days in each month?











    Results (378 votes), past polls