1
$tasks->{t1}{cmdline} = "convert -f %FONT% %1%";
$tasks->{t1}{report} = "Launched as %CMD% by %USER%";
$tid = 't1';
foreach my $parm (keys %{$tasks->{$tid}}) {
    $tasks->{$tid}{$parm}=~s/%FONT%/$tasks->{$tid}{font}/g; # may evaluate if $parm == 'cmdline';
    $tasks->{$tid}{$parm}=~s/%CMD%/$tasks->{$tid}{cmdline}/g;
}

The code is bad, because it needs another loop outside foreach (to ensure that all copied values were replaced), and also contains too much textual data. The best way to rewrite currently looks as:

foreach my $parm (keys %{$tasks->{$tid}}) {
 &lookupValues(\$tasks->{$tid}{&parm}); # or ($parm) if strict refs?
}

However it still needs to have keys sorted in order to detect priority (if %CMD% is replaced before %FONT%, $tasks->{$tid}{report} is wrong).

foreach my $parm ( &SORTME($tasks->{$tid}) ) {&lookupValuesFor($tid,$parm); }

What is a best way to sort depending on the number of specified variables and their relations? Native one (lots of loops and hashes), or expat [ Related questions are somehow pointing me to expat, would investigate it too: Parsing a string for nested patterns ] or other parser?

OOP way of $object->value('cmdline') is not liked now.

Community
  • 1
  • 1
kagali-san
  • 2,964
  • 7
  • 48
  • 87
  • It is not clear what you are worried about. You can reduce the number of `$tasks->{$tid}` references with: `$task = $tasks->{$tid}`; you can improve readability by including spaces around the '`=~`' operators. If the trouble is that you might do too much (or too little) expansion of strings that contain other '%XYZ%' values in the expansion, then you need to rethink how you handle the process altogether. Since it has taken an hour to get any feedback, you can reasonably take it that your question is somewhat inscrutable. – Jonathan Leffler Sep 26 '10 at 19:53

1 Answers1

2

This code 'works' - and looks cleaner to me. Whether it does what you're after is more debatable.

use strict;
use warnings;

my $tasks;

# Demonstration setup
$tasks->{t1}{cmdline} = "convert -f %FONT% %1%";
$tasks->{t1}{report}  = "Launched as %CMD% by %USER%";
$tasks->{t1}{maps} = {
    '%USER%'    => 'user-expansion',
    '%1%'       => 'one-expansion',
    '%FONT%'    => 'font-expansion',
    '%CMD%'     => 'cmd-expansion',
};

# Do the substitutions
foreach my $tid (keys %$tasks)
{
    my $task = $tasks->{$tid};
    my $maps = $task->{maps};
    foreach my $map (keys %$maps)
    {
        foreach my $key (keys %{$task})
        {
            next if ref $task->{$key};
            $task->{$key} =~ s/$map/$maps->{$map}/g;
        }
    }
}

# Demonstration printing
foreach my $tid (keys %$tasks)
{
    my $task = $tasks->{$tid};
    foreach my $key (keys %{$task})
    {
        next if ref $task->{$key};
        printf "%s: %-8s = %s\n", $tid, "$key:", $task->{$key};
    }
}

The output using Perl 5.13.4 on MacOS X (10.6.4) is:

t1: report:  = Launched as cmd-expansion by user-expansion
t1: cmdline: = convert -f font-expansion one-expansion

The triple loop is close to unavoidable; you want to apply each mapping to each string for each of the tasks. Each task can have its own mappings, in general.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278