Align text (left, center, right) without additional packages

Question

I wrote this short subroutine to left, center, right-align a multiline string.

How can I make it better (shorter, prettier, optimized)? Can you offer some suggestions?

#!/usr/bin/env perl
use strict;

# A bit of text
$_ = <<END;
     Lorem ipsum dolor sit amet consectetur adipiscing elit
          Curabitur pretium odio dictum nisl posuere, vitae mollis nibh facilisis
   Curabitur tempus tincidunt ante eget facilisis
     Pellentesque ut accumsan dui, nec semper odio
        Ut libero lacus, fermentum quis ultricies quis, tempus nec augue
                   Mauris tincidunt hendrerit accumsan
END

# Test each alignment case
for my $align ('left', 'center', 'right') {
    print align($_, $align)."\n";
}

# Alignment subroutine
sub align {
    local $_ = shift;
    my ($align) = @_;    # Text to align, Alignment type
    my $re = qr/^(.*)/m;
    s/^[ \t]+|[ \t]+$//gm;   # Remove trailing/leading spaces

    # Get longest line
    my $max = 0;
    length($1) > $max and $max = length($1) while /$re/g;

    if    ($align eq 'left')   { s|$re|$1 . " " x ($max-length($1))|ge }
    elsif ($align eq 'center') { s|$re|" " x (($max-length($1))/2) . $1|ge }
    elsif ($align eq 'right')  { s|$re|" " x ($max-length($1)) . $1|ge }

    return $_;
}

Answer 1

Welcome to Perl, and to Code Review :)

There are a couple of things that could be improved:

• It's good that you use strict. I recommend that you also use warnings.

• If you are using any recent Perl (say, anything since v5.10.1), then you can gain access to many convenient new features. You can do this by either specifying a minimum version (such as use v5.10) or by explicitly asking for a feature (use feature 'say'). The say feature is exactly like print, but appends a newline automatically.

• Please do not use here-docs like <<END. Always make it clear whether the here-doc behaves like a double-quoted string <<"END" or like a verbatim section <<'END'. Omitting quotes around the end marker behaves like double quotes, and can lead to bugs. Use single quotes if you don't want Perl to touch the string contents.

• Avoid the $_ variable whenever possible. Instead, use a variable with a good name such as $unaligned_text. As an aside, do not declare a $_ variable with my. This is currently still allowed, but will probably be forbidden in upcoming versions of perl:

  $_ is by default a global variable. However, as of perl v5.10.0, you can use a lexical version of $_ by declaring it in a file or in a block with my. […] Though this seemed like a good idea at the time it was introduced, lexical $_ actually causes more problems than it solves. If you call a function that expects to be passed information via $_, it may or may not work, depending on how the function is written, there not being any easy way to solve this. Just avoid lexical $_, unless you are feeling particularly masochistic. For this reason lexical $_ is still experimental and will produce a warning unless warnings have been disabled. As with other experimental features, the behavior of lexical $_ is subject to change without notice, including change into a fatal error.

  – from perldoc -v '$_'

• If you want to match any whitespace character in a regex, consider using the \s charclass rather than [ \t]. In this case, [ \t] may be preferable, but it will not match Unicode whitespace.

• Unless needed, do not put regexes into separate variables. Rather, specify them in-place. If you do put a regex object into a variable (which here makes sense, as you are reusing it), use a proper name, such as $match_line.

• Some regex modifiers change how a regex is compiled, others how a regex is matched. When creating a regex object with qr//, it is compiled. The /m flag changes the behaviour of the ^ and $ regex operators. By specifying this flag at the place of matching, you may be triggering multiple recompilations, slightly degrading performance. Instead: my $match_line = qr/.../m. The /g and /e flags change how matching and substitutions work, so they can't be used with qr//. For more information, see Regexp Quote-Like Operators in perldoc perlop.

• Instead of matching and changing each line, consider splitting the input into an array of lines. Then loop over each line and apply the required transformation. Afterwards, join the lines together again. This happens to avoid all that hassle with regexes.

• If an invalid alignment is passed, then you should throw an error rather than returning the input unaligned. I recommend you prefer the Carp module over the builtin die and warn.

Here is your align function, as I would write it.

sub align {
    my ($text_to_align, $alignment) = @_;
    my @lines = split /\n/, $text_to_align;

    # Trim whitespace, and find the longest line.
    # This is an OK use-case for the $_ variable.
    my $max_length = 0;
    for (@lines) {
        # I prefer \A and \z over ^ and $ because they always match at
        # string beginning and end, and are never influenced by /m.
        s{\A\s+}{};
        s{\s+\z}{};
        $max_length = length if length > $max_length;
    }

    # Perform the alignment, depending on the alignment type.
    # The alignment is performed in-place.
    if ($alignment eq 'left') {
        for my $line (@lines) {
            my $padding = $max_length - length $line;
            $line .= " " x $padding;
        }
    }
    elsif ($alignment eq 'right') {
        for my $line (@lines) {
            my $padding = $max_length - length $line;
            $line = (" " x $padding) . $line;
        }
    }
    elsif ($alignment eq 'center') {
        for my $line (@lines) {
            my $padding = $max_length - length $line;
            # we divide the $padding into two halves: $left and $right.
            # But what if $padding is an odd number?
            # Let's put the smaller padding $left.
            my $left = int($padding / 2);
            my $right = $padding - $left;
            $line = (" " x $left) . $line . (" " x $right);
        }
    }
    else {
        # this happens if an invalid $aligment was passed as argument.
        # Let's throw an error. We could use `die`, but that doesn't tell
        # an user of our function where *he* made an error. Instead, we use
        # the Carp module:
        require Carp;
        Carp::croak("Alignment must be one of left, right, center.");
    }

    # join the lines back together again, and return
    return join "\n", @lines;
}

Answer 2

I agree with @amon's answer for the most part. In addition, I propose an alternative implementation to eliminate this kind of duplicated logic:

if ($alignment eq SOME_ALIGNMENT) {
    for my $line (@lines) {
        my $padding = $max_length - length $line;
        $line = PADDED_LINE
    }
}

You can eliminate this duplication by putting the different alignment formatting functions into a map:

sub align_left {
    my ($line, $padding) = @_;
    return $line . " " x $padding;
}

sub align_right {
    my ($line, $padding) = @_;
    return (" " x $padding) . $line;
}

sub align_center {
    my ($line, $padding) = @_;
    # we divide the $padding into two halves: $left and $right.
    # But what if $padding is an odd number?
    # Let's put the smaller padding $left.
    my $left = int($padding / 2);
    my $right = $padding - $left;
    return (" " x $left) . $line . (" " x $right);
}

my %align_fn = (
    left => \&align_left,
    right => \&align_right,
    center => \&align_center,
);

This will simplify the main function a bit:

sub align {
    my ($text_to_align, $alignment) = @_;
    my $fn = $align_fn{$alignment} || do {
        # this happens if an invalid $aligment was passed as argument.
        # Let's throw an error. We could use `die`, but that doesn't tell
        # an user of our function where *he* made an error. Instead, we use
        # the Carp module:
        require Carp;
        Carp::croak("Alignment must be one of left, right, center.");
    };

    my @lines = split /\n/, $text_to_align;

    # Trim whitespace, and find the longest line.
    # This is an OK use-case for the $_ variable.
    my $max_length = 0;
    for (@lines) {
        # I prefer \A and \z over ^ and $ because they always match at
        # string beginning and end, and are never influenced by /m.
        s{\A\s+}{};
        s{\s+\z}{};
        $max_length = length if length > $max_length;
    }

    # Perform the alignment, depending on the alignment type.
    # The alignment is performed in-place.
    for my $line (@lines) {
        my $padding = $max_length - length $line;
        $line = $fn->($line, $padding);
    }

    # join the lines back together again, and return
    return join "\n", @lines;
}