kiat has asked for the wisdom of the Perl Monks concerning the following question:

Hi monks,

I'm wondering if there's a more elegant way of achieving what I'm doing with the code below (somewhat related to my node 366914): 

my $matrix_ref = [[1, a1, z1],[1, a1, z2], [1, a1, z3], [2, a2, z1], [
+2, a2, z2], [3, a3, z1]];

my ($html_str, $key, $count);

# id, a_item, z_item
foreach my $row (@$matrix_ref) {
  if ($row->[0] eq $key) {
    $html_str .= qq~<tr><td>$row->[2]</td></tr>\n~;
  }
  else {
    $html_str .= '</table>' if $count > 0;
    $html_str .= qq~<table><tr><td>$row->[0]</td></tr><tr><td>$row->[1
+]</td></tr><tr><td>$row->[2]</td></tr>\n~;
  }
  $key = $row->[0];
  $count++;
}

# output
1 a1 z1 z2 z3
2 a2 z1 z2
3 a3 z1

[download]
Lots of thanks in advance :)

Replies are listed 'Best First'.
Re: In seach of a more elegant foreach
by Zaxo (Archbishop) on Jun 16, 2004 at 01:54 UTC

    You have a natural refactoring, lets call it cell_wrap, 

    sub cell_wrap {
    join '', map {"<tr><td>$_</td></tr>"} @_;
}

    [download]
    Since you're just choosing between values in your conditional, the trinary op ?: comes to mind. Finally, I'd keep the foreach loop pretty much as you have it, though I'll pare it down to for and name no temporaries. 
    for (@$matrix_ref) {
    $html_str .= cell_wrap(
        $_->[0] eq $key
        ? $_->[2]
        : (@{$_}[0..2]) );
    $key = $_->[0];
}

    [download]

    It would be a bigger improvement to separate logic from presentation by merging the records and then feeding $matrix_ref to a template engine like Html::Template or TT2.

    Update: Arrgh! You changed your presentation code! Take this as another argument for templating.

    After Compline,
    Zaxo

[reply]
[d/l]
[select]
      Thanks, Zaxo!

      Your code looks so much more elegant :)

      Update: My apologies for changing the presenation code. I realised after I posted that I needed to put similar values in a table.

[reply]
Re: In seach of a more elegant foreach
by dragonchild (Archbishop) on Jun 16, 2004 at 02:31 UTC
    First off, there is a set of functions that will generate the HTML for you, in the CGI module. As it stands, your code doesn't emit well-formed HTML.

    But, to use that set of functions, you need to change your data structure into something that is more easily handled. Warning - the variables are poorly named. But, it is tested. 

    my @x;
my $curr = '';
my @curr;
foreach my $ref (@$matrix_ref)
{
    my $k = $ref->[0] . $ref->[1];
    unless ($curr eq $k)
    {
        push @x, [ @curr ] if @curr;

        @curr = @$ref;
        $curr = $k;

        next;
    }

    push @curr, $ref->[2];
}
push @x, [ @curr ];

use CGI qw( :all );

$html_str = table( map { Tr( td( $_ ) ) } @x );

    [download]

    ------
    We are the carpenters and bricklayers of the Information Age.

    Then there are Damian modules.... *sigh* ... that's not about being less-lazy -- that's about being on some really good drugs -- you know, there is no spoon. - flyingmoose

    I shouldn't have to say this, but any code, unless otherwise stated, is untested

[reply]
[d/l]
      Ah, now I see how I can cleanly separate code from html. Thanks, dragonchild!
[reply]
Re: In seach of a more elegant foreach
by cyocum (Curate) on Jun 16, 2004 at 11:51 UTC

    If you are handling matrices on a regular basis, I would use PDL. They have a bunch of modules for matrices and it might make your day a little easier.

[reply]
Re: In seach of a more elegant foreach
by diotalevi (Canon) on Jun 16, 2004 at 14:52 UTC
    I call shenanigans! You aren't using strict!
[reply]

Back to Seekers of Perl Wisdom