Re: string retrieval issue


Quoth mbliven1@xxxxxxxxx:

Paul Lalli wrote:
mbliven1@xxxxxxxxx wrote:
That's where I'm stuck. If the flat file looks like this:

Chicago Bears|NFC North
Detroit Lions|NFC North

then everything is great. However, if my file looks like this:
Chicago Bears|NFC North
Chicago Bulls|East Conference

then the script fails. In other words, if both lines begin with the
exact same string to the first space, then it will fail. It fails by
not writing the third element (Chicago Bulls) back to the array).
Ideas?

... You obviously did something wrong. How, precisely, are you
expecting anyone to tell you what you did wrong without seeing the code
you wrote?

Have you read the Posting Guidelines for this group? They're posted
here twice a week. I suggest you read them before replying.

My bad about the post. I have now read the guidelines.

....but seem to be unwilling to follow them.

Here is the code:


Where is

use strict;
use warnings;

?

open (FILE, "+<test.txt") || die ("test.txt wont open");

Use two-arg open.
Use lexical filehandles.
Say why the open failed.
The first rule of programming is: never type anything twice. Put the
filename in a variable.

my $database = 'test.txt';
open(my $FILE, '+<', $database) || die("'$database' won't open: $!");

If you use or instead of || those parens are optional:

open my $FILE, '+<', $database
or die "can't open '$database': $!";

but the code's OK like that if you find it easier to understand.

flock FILE, 2;

Don't *ever* use numeric constants for flock, seek, &c. Ask Perl to get
them right for you.
Check that you got the lock.

use Fcntl qw/:flock/; # this goes at the top

flock $FILE, LOCK_EX
or die "can't lock '$database': $!";

@lines = <FILE>;

You need to declare variables under 'use strict'.

my @lines = <$FILE>;

$j=0;
$k=0;

These would need declaring as well, except you very rarely need loop
counters in Perl. Perl knows how long your arrays are.

You do, however, need

my @splitdata;

foreach $element (@lines) {
^^ my

($element1, $element2) = split(/\|/, $element);

Variables with similar names are a red flag. They mean you should be
using data structures: in this case, an array.

my @elements = split(/\|/, $element);

@splitdata[$j] = $element1;

Don't use @ for accessing single array elements. It appears to work, but
it doesn't mean quite what you think it does. See perldoc -q @array.

$splitdata[$j] = $element1;

@splitdata[$k] = $element2;

This is pointless. $j and $k will always be equal, so you set the same
element twice. Every other element will be blank. I presume you meant to
set successive elements?

Use Perl's array operators, and then you don't need the temporaries
either:

push @splitdata, split /\|/, $element;

$j = $j + 2;

It's generally clearer to use the assignment operators where you can:

$j += 2;

, except, of course, that you don't need this at all.

$k=$k + 2;
}

foreach $p (param()) {

What is param? You didn't include it in your script. I'm going to guess
you meant to put

use CGI qw/:standard/;

at the top: when the posting guidelines say 'a short, complete script'
they mean you need to post the *whole* of your script. You also need to
provide example input: in this case, a query string.

Note that the order of parameters is not guaranteed to be the same as
the order of fields in the form. You should give your paramaters names
and access them by name.

$formvalue = param($p);
@formvalue[$i] = $formvalue;

if ($formvalue eq "") {
@final[$i] = @splitdata[$i];
}
else {
@final[$i] = $formvalue;
}

This can be written more simply as

$final[$i] = $formvalue || $splitdata[$i];

$i++;
}

I don't really understand what you are trying to achieve here. A
somewhat cleaner solution might be

my @params = params;
my %final;
$final{$_} = shift @splitdata for @params;
$final{$_} = param($_) for grep param($_), @params;

but I really think that a better answer would be to put the form field
names into your database. Do you have to use that file format? If you
could change to something like GDBM, then you could reduce the whole
script down to

use warnings;
use strict;

use GDBM_File;
use CGI qw/:standard/;

tie my %data, GDBM_File => './data', GDBM_WRCREAT;

$data{$_} = param($_) for grep param($_), params;

as GDBM handles locking for you. This will consider a parameter value of
'0' to be equivalent to '', so you may prefer

$data{$_} = param($_) for grep { length param($_) }, params;

It also assumes that your fields all have unique names; again, this may
not be the case, but you didn't provide us with any input data.

$x=0;
$y=0;
$z=0;

foreach (@final) {
$firstpart = @final[$x];
$secondpart = @final[$y];

Again, $x and $y are the same, so you're getting the same element twice.
Also, the loop loops once for every element in @final, but you're adding
2 to $x each time, so before long you'll be off the end of the array.

$writevalue = "$firstvalue|$secondpart";

What is $firstvalue? Is this your real script, or is that the bug you
were having trouble with? I can't see how it would produce the effect
you describe, though, unless this is not your whole script.

@finalwrite[$z] = $writevalue;
$x = $x + 2;
$y = $y + 2;
$z++;
}

seek FILE, 0, 0;

Don't use magic numbers, use constants.

use Fcntl qw/:seek/;

seek $FILE, 0, SEEK_SET;

truncate FILE, 0;

print FILE @finalwrite;

Since you haven't set $,, this will print the array elements all on one
line, separated by spaces. I don't think that's what you meant.

Ben

--
I must not fear. Fear is the mind-killer. I will face my fear and
I will let it pass through me. When the fear is gone there will be
nothing. Only I will remain.
benmorrow@xxxxxxxxxxxxx Frank Herbert, 'Dune'
.

Relevant Pages

  • Re: Logon script - function array and select case not working
    ... this all works well, except, the function i am using for the rules in the control script causes alot of querrys to AD. as there are alot of groups. ... objTSout.writeline retrv ... So if you think that this will assign an array value to the variable, how do you think the case select statement is going go compare this array value with the literal string values such as "group name here"? ... However, by not assigning ANY value to checkgrp in the function, you are guaranteeing that, should the function ever exit, it will return no information. ...
    (microsoft.public.scripting.vbscript)
  • Re: Perl: Subroutines and use of the "GD::Graph::bars;" Module
    ... I have been looking at the script once again, I could not really get your suggestion to work, but I have been doing some new changes thought. ... @array = sort @array; ... sub verbose { ... Global symbol "$title" requires explicit package name at ./bars.pl line 53. ...
    (perl.beginners)
  • Re: Perl: Subroutines and use of the "GD::Graph::bars;" Module
    ... I did what you told me, but I just gets this error msg in return when I am trying to execute the script: ... And I also get this error msg even when I am not commenting out the settings around line 93? ... @array = sort @array; ... sub verbose { ...
    (perl.beginners)
  • Re: settimeout needs alert() ???
    ... function slider { ... and use script to replace the src and title attributes. ... are downloaded completely. ... The usual strategy is to load all of the images in to an array of image ...
    (comp.lang.javascript)
  • Re: even rows for checkbox forms with no splitting of boxes from values
    ... 'mod' on the array length with a denominator equal to the width of the row, ... State saving really depends on what the script is already doing. ... So I usually roll my own checkboxes. ... #this is to create an array with 25-50 strings 2-10 in length ...
    (comp.infosystems.www.authoring.cgi)