CODEWITHSUNDEEP
perlif-statementfor-loopfilehandle
David
David

Reputation: 21

(empty?) return of readline is not caught by control structure

I have a multidimensional hash containing opened file handles on SEEK_END with the intention to always read the latest line without getting to much I/O (what I would get with tail).

I'm now going through all of these handles with a for loop and calling readline on them.

It looks like this:

for $outer ( keys %config ) {

    my $line = readline($config{$outer}{"filehandle"});

    if (not defined $line || $line eq '' ){
        next;
    }
    else{
        print "\nLine: -->".$line."<--\n";
        $line =~ m/(:)(\d?\.?\d\d?\d?\d?\d?)/;
        $wert = $2;
    }
}

If new content is written into these files, my script reads it and behaves just as planned.

The problem is that readline will usually return nothing because there is currently nothing at the end of the file, but my if doesn't seem to identify the empty return of readline as undef as empty -- it just prints nothing, which is would be right because there is nothing in this string, but I don't want it to be processed at all.

Upvotes: 2

Views: 77

Answers (2)

Sinan &#220;n&#252;r
Sinan &#220;n&#252;r

Reputation: 118118

I am a proponent of simple conditions. If it is at all feasible, I avoid compound logical conditions with an associated else so that I don't have to think about set complements etc. So, I would have listed the skip conditions individually:

next unless defined $line;
next unless $line =~ /\S/;

This kind of thing also tends to keep the number of nested blocks lower which I find improves the readability of code.

Upvotes: 1

Borodin
Borodin

Reputation: 126722

This is an operator precedence problem. You have used a mixture of low-priority not with high-priority || so your condition

not defined $line || $line eq ''

is parsed as

not(  defined($line)  ||  ($line eq '')  )

which wrongly negates the $line eq '' part

It is usually safer to use the lower-priority and and or, and not over &&, ||, and !, but a mixture is a very bad idea

You can write either

if (not defined $line or $line eq '' ) {
    ...
}

or

if ( ! defined $line || $line eq '' ) {
    ...
}

then all will be well

I would prefer to see it written like this, because it loses the unnecessary else clause and the next statements, and discards lines that contain just space characters

Also note that I iterate over the values of the hash. Using the keys is wasteful when they are used only to access the values. You will probably be able to think of a better name for the loop control variable $item

And there's often no need for the concatenation operator when Perl will interpolate variables directly into double-quoted strings

for my $item ( values %config ) {

    my $line = readline( $item->{filehandle} );

    if ( defined $line and  $line =~ /\S/ ) {

        print "\nLine: -->$line<--\n";

        $line =~ m/(:)(\d?\.?\d\d?\d?\d?\d?)/;
        $wert = $2;
    }
}

Upvotes: 6

Related Questions