in reply to perltidy block indentation

Apart from the style, why do you have an else block after return. I understand that you want to give an example of the style you require, but there is more than style: there should *NEVER* be an else after return, exit or die. These three exit the surrounding scope immediately, so the code *after* else is never executed. Retaining your style, that code should read:

sub old_code
    {
    my $self = shift;
    my $file = shift;
    
    if (-e $file) 
        {
        open(my $fh, "<", $file) or die "cannot open < $file: $!";  
        # Do something with the file
        return 1;
        }

    die "$file not found";
    };

[download]

Which I would simplify to

sub old_code
    {
    my $self = shift;
    my $file = shift;
    
    -e $file or die "$file not found";
 
    open(my $fh, "<", $file) or die "cannot open < $file: $!";  
    # Do something with the file
    return 1;
    }

[download]

Enjoy, Have FUN! H.Merijn

Replies are listed 'Best First'.
Re^2: perltidy block indentation
by dasgar (Priest) on Dec 04, 2013 at 15:07 UTC
    there should *NEVER* be an else after return, exit or die

    I'm not trying to say that you're wrong, but I'm personally struggling to understand the logic/rationale behind your claim. Based on the responses from Bloodnok and marto, there appears to be a good valid reason for doing as you are suggesting. I guess I haven't learned enough about Perl (and/or its internals) to understand what this reason is. Can you help me by providing more info on the rationale behind your suggestion so that I (and possibly others) can better understand your suggestion?

    Based on my current levels of knowledge and experience with Perl, I see the OP's code and your suggested code as being logically equivalent -- as in, both codes produce the same result. If I were responsible for maintaining the code, I would prefer the OP's code because I personally find it easier to read and follow and it will be less likely to cause me to misread what the code is doing.

    Again, I'm not trying to challenge you on this. I'm just trying to understand the logic and reasoning behind your view on what I would describe as a "best practice".

[reply]
[d/l]
[select]

      I'll try

      sub start_working_day
{
    if ($me->is_sick) {
        return undef;
        }
    elsif ($wheather->raining) {
        $travel->use_car;
        }
    else {
        $travel->use_bike;
        }
    $travel->go;
    $work->do;
    return;
    }

      [download]

      As you can see in this snippet, there are several expressions that control the start of a working day. After the if/elsif/else the real work starts, as it is the same for all situations: it is common code. Or is it? No it is not! When I am ill, the common code is never executed, so there is no reason whatsoever to have an else block: there is no need for an alternative, as there is no intent to run that common code.

      My stance is that a function or method should be written in a way that the most likely codepath is the easiest to follow with the eye. Exceptions should exit early.

      sub start_working_day
{
    $me->is_sick and return undef;

    if ($wheather->raining) {
        $travel->use_car;
        }
    else {
        $travel->use_bike;
        }
    $travel->go;
    $work->do;
    return;
    }

      [download]

      As you can now deduce, the code after the if/else is run for all cases of the decision tree, and hence reflects the thinking logic.


      Enjoy, Have FUN! H.Merijn
[reply]
[d/l]
[select]
Re^2: perltidy block indentation
by Anonymous Monk on Dec 03, 2013 at 12:42 UTC
    I am a fan of early exit and despise else-like structure as presented in OP myself. I don't know, however, the problems during a program run that caused you to mention "... should *NEVER* be an else after return, exit or die". Would you care to explain (how does the syntax in OP cause problem when a program runs)?
[reply]
      Whilst there are no run-time problems in the OPs' code per se, there are code static analysis tools e.g. B::Lint or Perl::Critic, that will complain/warn of unreachable code - of which the return statment in the OP is a perfect example, as described by Tux. This is best/most frequently addressed using the implied else construct - again something described in Tuxs' answer - in the 1st re-implemented code block.

      A user level that continues to overstate my experience :-))
[reply]
[d/l]

        Eh? There is no unreachable code in the OP. There is a non-coded space in the same scope as and below the if-else statement which *could* contain unreachable code, but there is NO code there. pertcritic confirms. 

        bash-4.2$ perlcritic --brutal pccdoe.pl 
Code is not tidy at line 1, column 1.  See page 33 of PBP.  (Severity:
+ 1)
Module does not end with "1;" at line 1, column 1.  Must end with a re
+cognizable true value.  (Severity: 4)
Code not contained in explicit package at line 1, column 1.  Violates 
+encapsulation.  (Severity: 4)
No package-scoped "$VERSION" variable found at line 1, column 1.  See 
+page 404 of PBP.  (Severity: 2)
Code before strictures are enabled at line 1, column 1.  See page 429 
+of PBP.  (Severity: 5)
Code before warnings are enabled at line 1, column 1.  See page 431 of
+ PBP.  (Severity: 4)
Found "\t" at the end of the line at line 5, column 1.  Don't use whit
+espace at the end of lines.  (Severity: 1)
Found "\N{SPACE}" at the end of the line at line 6, column 15.  Don't 
+use whitespace at the end of lines.  (Severity: 1)
Builtin function called with parentheses at line 8, column 3.  See pag
+e 13 of PBP.  (Severity: 1)
Close filehandles as soon as possible after opening them at line 8, co
+lumn 3.  See page 209 of PBP.  (Severity: 4)
"$fh" is declared but not used at line 8, column 8.  Unused variables 
+clutter code and make it harder to read.  (Severity: 3)
Useless interpolation of literal string at line 8, column 16.  See pag
+e 51 of PBP.  (Severity: 1)
"die" used instead of "croak" at line 8, column 31.  See page 283 of P
+BP.  (Severity: 3)
Magic punctuation variable $! used in interpolated string at line 8, c
+olumn 35.  See page 79 of PBP.  (Severity: 2)
Found "\N{SPACE}" at the end of the line at line 8, column 61.  Don't 
+use whitespace at the end of lines.  (Severity: 1)
"die" used instead of "croak" at line 14, column 3.  See page 283 of P
+BP.  (Severity: 3)

        [download]
[reply]
[d/l]

        There is no unreachable code.

        But the '-e' (test if files exists) is unnecessary, because the 'open ... or die' includes the case of a not existing file.

[reply]
Re^2: perltidy block indentation
by Anonymous Monk on Dec 03, 2013 at 12:14 UTC

    Well, thank you for that code review! Please note that your simplified code fails to communicate the style points I was seeking wisdom on...

[reply]

      They said "Apart from the style", they're not addressing the style issue but a more serious aspect of the code you posted.

[reply]

In Section Seekers of Perl Wisdom