Could someone help me in undertanding a piece of code? 1

[whn]

I inherited a piece of code that I could not fully understand:

sub load_scenarios {
  my ($tests) = @_;
  my @results;
[COLOR=red]
  my $add_instance = sub {
    my ($name, $parent) = @_;[b]
    my $instance = eval "my \$inst = new $name; \$${name}::parent = '$parent'; \$inst";
    not $@ or die "Can't create instance for $name: $@\n";[/b]
    push @results, $instance;
  };
[/color]
  foreach my $test (split /,/, $tests) {
    # Some Implementation here
    if ($test =~ m~^(.+)/(UserScenario\w+)$~) {
      my ($subdir, $US) = ($1, $2);
      load_module $test;[COLOR=blue]
      &$add_instance($US, $subdir);[/color]
    }
    # Some Implementation here
  }
  return @results;
}

I know the red part is invoked by the blue part. But I don't quite understand what technique is this. And I don't understand exactly how those two BOLD RED lines work. Is there a specific TERM for this kind of implementation (such as call-back)? Is there a better way (more efficient and easier to understand) to implement this? Or is there an alternative way to implement this, though the alternative way could be less efficient but easier to understand?

Thank you very much for your help.

 

[MillerH]

To put it simply, this is messy code.

$add_instance contains a reference to a subroutine. This sub is a closure on the local variable @results. If $add_instance isn't used anywhere else, I would kill it. Also, the fact that it uses a closure, instead of simply returning the new $instance and letting the caller add it to the @results array is silly.

As for what the add_instance sub is doing, it's creating a new object for every UserScenario* in the $tests lists. It's then setting the package variable $parent to the subdirectory where the module is located, but we won't be able to determine why it does this without looking at the modules themselves.

So, assuming that $add_instance isn't used anywhere else, this is how I would provisionally clean up the code. Even if it is used elsewhere, I would remove the closure aspect to it, make it an named sub outside of load_scenarios, and do the pushing on @results in the proper place.

[url=http://perldoc.perl.org/functions/sub.html][black][b]sub[/b][/black][/url] [maroon]load_scenarios[/maroon] [red]{[/red]
	[url=http://perldoc.perl.org/functions/my.html][black][b]my[/b][/black][/url] [red]([/red][blue]$tests[/blue][red])[/red] = [blue]@_[/blue][red];[/red]
	
	[black][b]my[/b][/black] [blue]@results[/blue] = [red]([/red][red])[/red][red];[/red]

	[olive][b]foreach[/b][/olive] [black][b]my[/b][/black] [blue]$test[/blue] [red]([/red][url=http://perldoc.perl.org/functions/split.html][black][b]split[/b][/black][/url] [red]/[/red][purple],[/purple][red]/[/red], [blue]$tests[/blue][red])[/red] [red]{[/red]
		[gray][i]# Some Implementation here[/i][/gray]
		[olive][b]if[/b][/olive] [red]([/red][blue]$test[/blue] =~ [red]m~[/red][purple]^(.+)/(UserScenario[purple][b]\w[/b][/purple]+)$[/purple][red]~[/red][red])[/red] [red]{[/red]
			[black][b]my[/b][/black] [red]([/red][blue]$subdir[/blue], [blue]$US[/blue][red])[/red] = [red]([/red][blue]$1[/blue], [blue]$2[/blue][red])[/red][red];[/red]
			
			[maroon]load_module[/maroon][red]([/red][blue]$test[/blue][red])[/red][red];[/red]
			
			[black][b]my[/b][/black] [blue]$instance[/blue] = [url=http://perldoc.perl.org/functions/eval.html][black][b]eval[/b][/black][/url] [red]"[/red][purple]my [purple][b]\$[/b][/purple]inst = new [blue]$US[/blue]; [purple][b]\$[/b][/purple][blue]$[/blue]{US}::parent = '[blue]$subdir[/blue]'; [purple][b]\$[/b][/purple]inst[/purple][red]"[/red][red];[/red]
			[olive][b]if[/b][/olive] [red]([/red][blue]$@[/blue][red])[/red] [red]{[/red]
				[url=http://perldoc.perl.org/functions/die.html][black][b]die[/b][/black][/url] [red]"[/red][purple]Can't create instance for [blue]$US[/blue]: [blue]$@[/blue][purple][b]\n[/b][/purple][/purple][red]"[/red][red];[/red]
			[red]}[/red]
			
			[url=http://perldoc.perl.org/functions/push.html][black][b]push[/b][/black][/url] [blue]@results[/blue], [blue]$instance[/blue][red];[/red]
		[red]}[/red]
		[gray][i]# Some Implementation here[/i][/gray]
	[red]}[/red]
	
	[url=http://perldoc.perl.org/functions/return.html][black][b]return[/b][/black][/url] [blue]@results[/blue][red];[/red]
[red]}[/red]

There probably is a way to clean up the creation of the objects, but without working with the real code, it would be unwise for me to suggest alternative solutions.

- Miller