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

Ok I posted a script similar to this and fixed it up. I tested it but only on gold and that worked. The only problem is that all the $PasswordLine etc. have the value for gold. Can you please tell me what is wrong I have been working on it for hors now and am giving up hope. But I need it done soon for a school project
#!/usr/local/bin/perl use CGI ':standard'; print header(); print start_html(); open FILE, "user/test"; for $line (<FILE>) { print index $line,"password";$PasswordLine=$line; if (index $line, "password" == 0) { } if (index $line, "minatt" == 0) { $MinattLine=$line; } if (index $line, "maxatt" == 0) { $MaxattLine=$line; } if (index $line, "exp" ==0 ) { $ExpLine=$line; } if (index $line, "gold" == 0) { $GoldLine=$line; } } close FILE; sub getpassword { my $start= index $PasswordLine, ":"; return substr ($PasswordLine, $start + 2, length ($PasswordLine)) +; } sub getminatt { my $start=index $MinattLine, ":"; return substr $MinattLine, $start+2, length ($MinattLine); } sub getmaxatt { my $start= index $MaxattLine, ":"; return substr $MaxattLine, $start+2, length $MacattLine; } sub getexp { my $start= index $ExpLine, ":"; return substr $ExpLine, $start+2, length $ExpLine; } sub getgold { my $start= index ($GoldLine, ":"); return substr $GoldLine, $start+2, length $GoldLine; } print getgold; print getexp; print getmaxarr; end_html();

Replies are listed 'Best First'.
Re: problem with variable values
by Albannach (Monsignor) on Jun 13, 2001 at 07:36 UTC
    Your problem is that the precedence of == (numeric equal) is higher than that of , (comma). This means that the numeric comparison happens before the call to index, so every one of your index calls is returning the position of 1 in the string (numeric comparison of any string with 0 yields true or 1), and unless the string actually contains 1, your if( index... lines will all return -1, which if interprets as true. Unfortunately if the string does contain 1, index will return the postion of the digit, which will also be seen as true by if unless the digit happens to be at position 0 (first in the string).

    To make a long story short, all your data match all your conditionals and so all your variables are set to each line. Since your "gold" line is last in your data, all the variables have been set to that line by the time your script produces output. To change this, you can force the precedence you want using brackets in the appropriate locations.

    All of that however is evading many handy Perl features. One easier way to do it which is still similar to yours would be to use perlfunc:split to extract your keywords and values from your data lines, along the lines of:

    my($keyword,$value)= split ':', $line;

    as the first line in your loop, then just test the possible values of $keyword. There are many "more Perlish" ways to do this, but this should suffice without getting you too confused for now.

    Finally, you should always use strict, and check the return value when you open a file. I hope this helps!

    --
    I'd like to be able to assign to an luser

Re: problem with variable values
by bwana147 (Pilgrim) on Jun 13, 2001 at 16:58 UTC

    Ok, just not to let you think that PerlMonks are not helpful people, I'll go on with Albannach's recommendations and propose a more Perlish way of doing it.

    First off, you should use strict. This prevents you from using undeclared variables, which is usually considered bad programming practice. use strict is not enabled by default because Perl is polite enough to not dictate what good programming practices are.

    Then, look at your subs. They all rely on some global variable being set. This is also generally considered as bad programming practice. (UPDATE, following sierrathedog04's remark: of course, this only true in general. There are cases where global variables come in handy. They should normally be kept to a minimum) A function should take arguments and return a result or perform an action that depends solely on these arguments. This allows the code to be more generic and robust. In that case, all your getsomething functions could have been only one function:

    sub getvalue { my $line = shift; # get the argument my $start = index($line, ':'); return substr($line, $start+2, length($line)); }

    Which you would call like so:

    print getvalue($PasswordLine); print getvalue($MinattLine); print getvalue($MaxattLine); print getvalue($ExpLine); print getvalue($GoldLine);

    Actually, there are other ways to achieve this. You should read the substr manpage to learn a more elegant way of getting the last part of a string, and the split manpage for an even more elegant way of splitting a string into parts. With this new knowledge, you'll find it's so easy that you won't want to create a function for such a trivial task.

    Back to your problem... So, you want to open a file, the lines of which consist of a key and a value, separated by a colon. Once the file has been read through, you want to retrieve the values that were associated with the keys. Keys, values, hmmm, that sounds like a job for hashes.

    Here you go: open the file and complain if something goes wrong:

    open FILE, "user/test" or die "Error: $!\n";

    Loop over each line, and close the file at the end:

    while ( my $line = <FILE> ) { ... } close FILE;

    Each line consists of a key/value pair, separated by a colon and possibly whitespace. We will store the data into a hash, which you need to declare before the while loop.

    my %data = ();

    Foreach line, we'll split it on the colon, possibly with whitespace (read perlre to learn everything about regular expressions). But you don't want the trailing newline character to be part of the value, do you? Use chomp for that:

    chomp $line; my ($key, $value) = split(/\s*:\s*/, $line);

    Now, stuff the value into the hash, indexing it with the key:

    $data{$key} = $value;

    When the loop is over, you can access the elements separately from %data:

    print $data{password}; print $data{minatt}; print $data{maxatt}; print $data{exp}; print $data{gold};

    Here's the code in its entirety:

    #!/usr/local/bin/perl -w use strict; open FILE, "user/test" or die "Error: $!\n"; my %data = (); while ( my $line = <FILE> ) { chomp $line; my ($key, $value) = split(/\s*:\s*/, $line); $data{$key} = $value; } close FILE; print $data{password}; print $data{minatt}; print $data{maxatt}; print $data{exp}; print $data{gold};

    Of course, since you seem to be sending all that to a web browser, you'll need a little extra formatting. But I won't do all the work for you! Raise your heart and good luck in your quest...

    --bwana147

      A function should take arguments and return a result or perform an action that depends solely on these arguments.
      My own view is less extreme. For instance, the standard way to produce a variable in a subroutine that remembers its values between calls is as follows:
      use strict; # tested on 5.6/win print mysub(); print "\n" . mysub(); {my $persistentVariable; sub mysub { $persistentVariable++; } }

      There are other uses as well for declaring a variable used by a subroutine outside of that subroutine. For instance, since an anonymous subroutine is only compiled once, any such variable when called in an anonymous subroutine will preserve whatever value it had at the time the subroutine was declared.

        The issue that you are fighting is locality of reference. Locality of reference means that any given variable is localized as much as possible to a small section of code. See Pass by reference vs globals or RE (tilly) 3: redeclaring variables with 'my' for my attempts to explain why this matters.

        The iron-clad rule is just a rule that improves locality of reference. But the rule doesn't start or end there. For instance the other day I had an array of colours that I needed to synchronize between a table and a graph. Now I could easily export the array I used in my graph to the table and it would work. But that wouldn't preserve locality of reference, someone else might do something stupid like put in code somewhere that set that array making it hard to figure out why the colors did not synchronize properly.

        So I wrote a function in the graph code that returned a closure which would take in elements going into the table and return them with the appropriate color tag. Much better! Inside the graph code you can see that nobody else is abusing this array. Inside the function there is hidden state, but that is documented and the table code knows all of the calls to that function. (Why didn't I return the array? Because if you run out of colors you cycle, and I found that implementing that bit of internal logic in the table to not be very nice. Besides which, some day that behaviour might change, and it would be good that I have it localized!)

        So I had an example where I wrote almost exactly what you wrote above. Which breaks the rule that people learn. But by breaking the rule I believe I managed to achieve the goal of the naive rule...

      Thank you.Thank you. Thank you. Thank you.
      Now what I need is a redirect link. I was using.

      for $line (<FILE>) { print $line; } print $data{
      but it will not work for redirecting to other cgi files

Re: problem with variable values
by Anonymous Monk on Jun 13, 2001 at 07:13 UTC
    As you can tell their is a copying error it should be
    if (index $line, "password" == 0) { %PasswordLine=$line; }

      the file it is reading from looks like this
      name: jeff
      password: jeff
      minatt: 2
      maxatt: 15
      exp: 0
      gold: 10