Beefy Boxes and Bandwidth Generously Provided by pair Networks
"be consistent"
 
PerlMonks  

Re^4: What's the right way to write a method which returns one line at a time from a file?

by jcb (Parson)
on Nov 23, 2020 at 03:24 UTC ( [id://11124045]=note: print w/replies, xml ) Need Help??


in reply to Re^3: What's the right way to write a method which returns one line at a time from a file?
in thread What's the right way to write a method which returns one line at a time from a file?

That is ridiculous: the else block is only executed if the condition on the if is false. Since the return is in the if block, it is only executed if the condition tested evaluates to true, which means that the else would never be executed anyway.

This is a perfectly legitimate construct when different return values must be produced in different conditions, or when an early exit is needed under some conditions.

Replies are listed 'Best First'.
Re^5: What's the right way to write a method which returns one line at a time from a file?
by Tux (Canon) on Nov 23, 2020 at 07:57 UTC

    It is not what is executed in the else block, but what is execute after.

    if (expression) { statement (1); } else { statement (2); } statement (3);

    If statement (1) exits the enclosing scope with return/exit/die/croak, statement (3) is never executed. An if/else is the to do something different based on a condition and then continue doing what is after the construct for all cases. This implies that an else immediately after return/exit/die/croak is always useless.

    The original code does it right in the first block:

    sub get_filehandle { my $self = shift; $self->{file} = shift; open my $fh, "<", $self->{file} or die "can't open $self->{file}"; return $fh; }

    Which - when following the wrong method you approve of - would be

    sub get_filehandle { my $self = shift; $self->{file} = shift; if (open my $fh, "<", $self->{file}) { return $fh; } else { die "can't open $self->{file}"; } }

    In my way, the second block should be reduced to

    sub get_lines { my $self = shift; $self->{file} = shift; ### get the filehandle if we don't already have one $self->{file_handle} ||= $self->get_filehandle ($self->{file}); return readline ($self->{file_handle}) || undef; }

    That last return is arguable anyway, as the readline method can be a method that legally can return undef, "", and 0 (or any object evaluating as false) indicating that the original code was wrong to start with.


    Enjoy, Have FUN! H.Merijn

      While statement (3) in your example is dubious, statement (2) and its else block are completely legitimate if statements (1) and (2) both cause early exit from the block. If statement (2) does not cause an early exit, the else should be removed (or changed to a comment # else ...) and statement (2) promoted to the same position as statement (3), leaving statement (1) as a conditional early exit. If statements (1) and (2) both cause early exits, statement (3) is dead code and should be removed.

      I think I see your rationale, in that an else block can be redundant in these situations, since the entire rest of the block after a conditional early exit is effectively an "else" block without the additional indentation level. I still say that a rule that else should never be used in this context is ridiculous because it can be used to cause the indentation to line up and produce clearer code, particularly in the case where one of two (or of multiple, if elsif is used) possible return values should be chosen. In such a context, using the final else allows all of the return statements to line up at the same indentation level, producing clearer code, especially if K&R brace placement is used.

        You're almost in my lane :)

        The key for my claim is that the return/exit/die/croak is the last unconditional statement in the if block. I might have been clearer on that.


        Enjoy, Have FUN! H.Merijn

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://11124045]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others musing on the Monastery: (6)
As of 2024-03-29 00:05 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found