Hello Jay
Jay Savage wrote:
> On 7/25/06, Rob Dixon <[EMAIL PROTECTED]> wrote:
>
>>
>> my @values;
>>
>> while (<$fh>) {
>> next unless /(\S+)\s*=\s*(\S+?)\s*;?/;
>> push @values, $1, $2;
>> }
>>
>> @values;
>> }
There is a problem with my code which means that it will return only the first
character of multi-character parameter values. It suffices for Siegfried's case
where the values where just '1' but it is flawed, and much better would have
been:
next unless /(\S+)\s*=\s*(\S+?)\s*;?\s*$/;
which pulls in all of the value and allows for an optional trailing semicolon
with optional whitespace on either side.
>
> Be careful with this. With regex, it's (usually) better to seach for
> the things you want than to try to exclude the things you don't. There
> are a whole lot of things in \S that we probably don't want here.
The OP was using a text file with just two numeric values and wanted something a
little more like a proper initialisation file with labelled values. The
suggestion you make is certainly valid, but there is always room to enhance a
solution without necessarily making it more useful.
> If you want numbers, look for numbers. Because if there's a typo in the file,
> Perl will quite happily go on and use non-numeric matches of \S in subsequent
> arithmatic, yeilding unexpected and difficult to debug results, e.g.
>
> $a = "F";
> print $a + 0;
What your attribution above doesn't show is that this is a general purpose
subroutine which takes a filename as its parameter and returns a hash value
describing all lines in that file looking roughly like
param = value
The intention was that, once the data was in a Perl data structure, the calling
code could make sure that the required values were both present and valid. In
the case of your given data subroutine would return a list that, when assigned
to a hash, would look like:
my %params = ('$a' => '"F"');
and the second line would be ignored as it contains no '=' character. It was
never my intention to parse Perl source. Usage would be something like
my %params = getParams('delay.txt');
my $delay = $params{delay} || '1';
die 'Invalid delay value' if $delay =~ /\D/;
my $continue = $params{continue} || 'yes';
die 'Invalid continuation setting' unless $continue =~ /^(yes|no)$/;
depending on what was wanted. It would certainly be possible to make the
subroutine more intelligent by, say, stripping quotes from a parameter value or
forcing numeric data, but I say that would be unnecessary and against the
principle of writing something functional but concise. It is not for the slave
to decide what his master will eat, simply to offer him what is available.
> Using $1 and $2 in this context is a bit redundant, too. instead, try
> something like
>
> my ($condition, $value) = /\s*(\w+)\s*=\s*(\d+)/;
This is nonsense. You choose to declare and assign two additional scalar
variables with the same contents as the already-existing $1 and $2, just to push
them onto an array and discard them again? It is your alternative that is
redundant. And look at the ugly code you get if you try to both validate and
parse the record in the same statement as did my 'next unless //'. It would be
nice to see what a full alternative solution would look like using your ideas.
As I have said, enhancements are always possible but not always desirable. Such
ideas may be useful to the group, but please post them as suggestions rather
than as corrections to previous posts.
Cheers,
Rob
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>
