Spenser has asked for the wisdom of the Perl Monks concerning the following question:

I have what I think is a scoping problem with a set of two foreach statements.  I have two tables in mySQL:  one with employee info., in particular employee id's and employee names;  the other table has information on each employee's work schedule for each day.  I'm trying to loop through the list of employees and then for each employee, loop through the schedule records for each day relavent to each employee.  Note, there are only five records for each employee since we only schedule one work week at a time.

Below is an excerpt of my code.  I tried to cut it down to only the relevant pieces.  So, please fill in the missing pieces with your imagination.

# After the initial, usual stuff I query mySQL using
# DBI and extract employee id numbers and their names
# for an array and a hash.

while (@emp_list = $sth->fetchrow_array()) {
   push(@emps, $emp_list[0]);
   $employees{$emp_list[0]} ="$emp_list[1]"; }

$sth->finish();

my @days = ('Mon', 'Tue', 'Wed', 'Thu', 'Fri');

foreach $emp_id(@emps) {
   print "Employee: ", $employees{$emp_id}, "\n";
   print "Employee ID: ", $emp_id, "\n";

   foreach $wkday(@days) {
       $sql_str = "SELECT emp_name, field2, field3
                   FROM table1
                   WHERE emp_id='$emp_id'
                   AND wkday='$wkday'";
        $sth= $dbh->prepare($sql_str);
        $sth->execute();
        @schedule = $sth->fetchrow_array();  
   }
        print @schedule;
# Actually I break the @schedule up nicer than this.

}

[download]

The problem seems to be that the value of the variable $emp_id does not penetrate the second or inner foreach loop.  The result is that $emp_id for the loop regarding the schedule is blank, although the $emp_id does print on the header line above it.  How do I pass the value of a variable (e.g., $emp_id) of one foreach loop onto another foreach loop?

That's Spenser, with an "s" like the detective.

Replies are listed 'Best First'.
Re: foreach within a foreach
by shotgunefx (Parson) on Jun 07, 2002 at 19:32 UTC
    It doesn't appear to be a scope or assignment problem.

    Try adding a print "inner: $emp_id\n" as the first statement inside the foreach.

    Also, you should use placeholders inside the SQL statement. 
    $sql_str = "SELECT emp_name, field2, field3
                   FROM table1
                   WHERE emp_id=?
                   AND wkday=?";

$sth = $dbh->prepare($sql_str);
$sth->execute($emp_id,$wkday);

    [download]


    -Lee

    "To be civilized is to deny one's nature."
[reply]
[d/l]
[select]
      Just out of curiousity, and not in argument, because I do not know the answer to the question.

      But why, particularly, are placeholders better than using your own variable?
[reply]
        Placeholders make sure that the data is quoted correctly. Remember, when inserting strings into your database, they must be surrounded by quotes, yet numbers cannot be. This strict either/or but not both situation means that you need to decide in advance how to quote. If you put quotes in your statement, but change data types later, you are going to break that part of your program in some way that isn't visible until that statement is run.

        The other thing you can do is let DBI do it for you, which is what placeholders do. DBI keeps track of how each column has to be treated, and quotes accordingly. As an additional bonus, some driver implementations, such as DBD::mysql can actually save these generic statements and recycle them later. In shotgunefx's example, the execute could be called many times on exactly the same prepared statement.
[reply]
        Efficiency. If you use placeholders, you only need to prepare() the statement once (at which point the db engine analyzes it and figures out how to most efficiently process the query) and can then execute it repeatedly, passing in different parameters each time. Without placeholders, the query has to be re-prepared each time it is executed.
[reply]
[d/l]
        Well depending on what's in your $vars, it could break your SQL if it contains ? or a quote. more on this

        The other reason is that you don't have to keep preparing a statement over and over. The poster could move the prepare outside of both foreach loops and only prepare it once. Over thousands of iterations, this can make a noticable difference.

        -Lee

        "To be civilized is to deny one's nature."
[reply]

      Well, I did as you suggested, shotgunefx:  I added a print statement after the first line of the inner foreach statement and set up place holders.  The result was that I could see the employee id printed five times (once for each weekday record) for each employee.  So the inner foreach statement does seem to be getting the employee id numbers.

      I tested a little further by printing out @schedule at the end of the inner loop and I ended up with data displayed that was lengthier with each looping.  It turns out that the problem was with a push statement that I didn't copy into my posting.  In an attempt to shorten my posting to make it easier to discuss, I didn't re-type my code accurately.   The last line I presented in the inner loop reads on the posting as follows:

      @schedule = $sth->fetchrow_array();

      [download]

      In actuallity it reads in my script like this:

      @_ = $sth->fetchrow_array();
push(@schedule, @_);

      [download]

      The problem is that the first employee has nothing scheduled because she's on vacation this week.  And since I'm appending the array for each record and not resetting it after each inner loop, each employee looks like they have nothing scheduled.

      I've gone back and added @schedule = ""; to the line just above the inner loop and it now works.  Sorry I didn't post that last line correctly--as I said I was looking for brevity in the code at least.  Thanks for the troubleshooting suggestion regarding putting print statements in the inner loop.  That brought the whole thing to light.

      That's Spenser, with an "s" like the detective.

[reply]
[d/l]
[select]
        Glad I could help. A couple comments though, you probably want to initilize @schedule with an empty list. 
        @schedule = ();

        The other is you usually don't want to assign to @_, depending on the context it might cause subtle bugs. @_ has some magic associated with it. For instance, in a sub, @_ acts as aliases to the passed arguments. (Example below)

        It appears assigning to @_ makes it lose it's magic so I don't think it is a problem here but it's probably a dubious habit to form. 

        #!/usr/bin/perl -w

use strict;

my ($foo,$bar,$baz) = (1,2,3);

print "$foo, $bar, $baz \n"; # Prints 1, 2, 3

foo($foo,$bar,$baz);

print "$foo, $bar, $baz \n"; # Prints 5, 10 , 15


sub foo {
    foreach (@_) {
        $_ *= 5;
    }
}

        [download]
        -Lee

        "To be civilized is to deny one's nature."
[reply]
[d/l]
Re: foreach within a foreach
by rbc (Curate) on Jun 07, 2002 at 21:15 UTC
    You should take a look at your SQL.
    You might want to join your employee table
    and your emp_name table. Also you could avoid
    looping over @days by changing your where
    clause to have something like this in it
     AND wkday in ( 'Mon', 'Tue', 'Wed', 'Thu', 'Fri')
[reply]
[d/l]

      That "IN" is a good idea.  I tried it just now and it helped to cut down on a lot of code, but eliminating one layer of my foreach statements caused other problems.

      I think have so much else going on in this script and even within the two foreach loops (lots of stuff that I left out of the posting) that it would be too much to unravel.  I may go back later and use it on this script.  I especially would like to adjust some other scripts that I created with this "IN" command.  I wasn't happy with the awkwardness of the code that I created by doing loops within loops.

      I've been learning that a good mySQL statement can significantly cut down on a lot of Perl coding and speed up the programming process. Thanks for the tip.

      That's Spenser, with an "s" like the detective.

[reply]

Back to Seekers of Perl Wisdom