1. Indent data structures and arguments to subs rather than leaving them hanging off to the right:

sub ugly_formatting {
    my %conn_data  = @_;
    my $dbh = DBI->connect( $conn_data{ data_source }, $conn_data{ use
+r }, $conn_data{ pass },
                            { RaiseError  => 1,
                              AutoCommit  => 0,
                              LongReadLen => 65_634 } ) or die DBI->er
+rstr;
    $dbh;
}
[download]

Instead, I do this:

sub _connect 
{
    my %conn_data  = @_;
    my $dbh = DBI->connect( 
        $conn_data{ data_source }, 
        $conn_data{ user }, 
        $conn_data{ pass },
        {
            RaiseError  => 1,
            AutoCommit  => 0,
            LongReadLen => 65_634 
        } 
    ) or die DBI->errstr;
    $dbh;
}
[download]

2. Curly braces should be aligned vertically over themselves on the same scope, rather than trailing off the end of a line. Quick, where's the missing brace:

if ( exists $database->{ $table }{ _unique } ) {
    my @unique_fields = @{ $database->{ $table }{ _unique } };
    foreach my $field ( @unique_fields ) {
        my %field_count;
        my $sql = "SELECT $field FROM $table";
        my $fields = $dbh->selectall_arrayref( $sql );
        foreach ( @$fields )
            $field_count{ $_->[0] }++;
        }
        foreach ( keys %field_count ) {
            if ( $field_count{$_} > 1 ) {
                print $err "Table: '$table' Unique field: '$_' was fou
+nd $field_count{$_} times\n";
                $error_count++;
            }
        }
    } # next $field
}
[download]

If you vertically align corresponding braces:

if ( exists $database->{ $table }{ _unique } ) 
{
    my @unique_fields = @{ $database->{ $table }{ _unique } };
    foreach my $field ( @unique_fields ) 
    {
        my %field_count;
        my $sql = "SELECT $field FROM $table";
        my $fields = $dbh->selectall_arrayref( $sql );
        foreach ( @$fields )
            $field_count{ $_->[0] }++;
        }
        foreach ( keys %field_count ) 
        {
            if ( $field_count{$_} > 1 ) 
            {
                print $err "Table: '$table' Unique field: '$_' was fou
+nd $field_count{$_} times\n";
                $error_count++;
            }
        }
    } # next $field
}
[download]

To me, the problem just leaps out at me. As I sometimes wind up with really complex structures, vertical alignment of the braces seems to help.

3. Indentation can start with tabs for leading whitespace, but embedded whitespace must be spaces.

That might sound a bit weird, but many editors have tabs at different settings. Maybe they're 8 spaces. Maybe they are 4 or 2 (I've heard rumors of them viewing as 6 spaces, what heresy!). By ensuring that only leading whitespace (/^\s+/) is tabs (spaces are okay, too), but all embedded whitespace is comprised of spaces, you ensure that code won't wind up with a bizarre visual appearance, depending on the editor. For example, I recently encountered the following code (tabs are changed to spaces so you can see what I mean):

my %page_control = ( Add    => { EmploymentListing => { page     => 'e
+m-main-add-employment.tmpl',
                                                 function => \&add_emp
+loyment }
                               },
                     Edit   => { EmploymentListing => { page     => 'e
+m-main-update-del-employment.tmpl',
                                                  function => \&update
+_delete_employment }
                               },
                     Delete => { EmploymentListing => { page     => 'e
+m-main-update-del-employment.tmpl',
                                                  function => \&update
+_delete_employment }
[download]

Now, that's pretty ugly, but that's with tabs rending four spaces in width. I opened this up to work on it in an editor that renders tabs as being eight spaces wide:

my %page_control = ( Add    => { EmploymentListing => { page     => 'e
+m-main-add-employment.tmpl',
                                                                      
+                           function => \&add_employment }
                                                           },
                                         Edit   => { EmploymentListing
+ => { page     => 'em-main-update-del-employment.tmpl',
                                                                      
+                            function => \&update_delete_employment }
                                                           },
                                         Delete => { EmploymentListing
+ => { page     => 'em-main-update-del-employment.tmpl',
[download]

That's an even worse mess. I took some time to convert it:

my %page_control = ( 
    Add => 
    { 
        EmploymentListing => 
        { 
            page     => 'em-main-add-employment.tmpl',
            function => \&add_employment 
        }
    },
    Edit => 
    { 
        EmploymentListing => 
        { 
            page     => 'em-main-update-del-employment.tmpl',
            function => \&update_delete_employment 
        }
    },
    Delete => 
    { 
        EmploymentListing => 
        { 
            page     => 'em-main-update-del-employment.tmpl',
            function => \&update_delete_employment 
        }
    }
};
[download]

Note that rule 3 doesn't do that much good if rule 1 is ignored.

Now, here's the sad truth: I don't like this. Somehow, it grates against my sense of aesthetics. So why am I doing it? Because I think that in the long run, code is much cleaner and easier to read, regardless of the editor one chooses. Further, I feel that scoping issues will be easier to track down like this.

Here's the rub: is enshrining this into standards the way to go? I think many programmers would rebel. I really don't think we'll get much benefit. grep wants the standards, though, to ensure that we don't wind up with a bunch of different programmers reformatting everyone else's code every time they work on it (yes, I'm guilty of that :). Any thoughts?

Cheers,
Ovid

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.