Please help.

I'm not sure I can help you solve your problem as it seems you've left out some of the code that produces the result. How does your $List variable get populated, for instance?

I can make some suggestions that might help you clean up your code regardless of whether they fix your problems.

• Don't use prototypes unless you know what you are doing. By the way, you don't know what you are doing until you understand the points raised in Tom Christiansen's article.

• Get rid of your isDefined() subroutine. All it is doing is allowing you to replace

  defined $hash->{$key}
  [download]

  with

  $self->isDefined($hash, $key)
  [download]

  which is longer, less clear, and less efficient. Even if it made sense to use such a generic utility function (and in your case, it doesn't) it does not make sense to make it a method unless it does something specific to your class.

• Consider using [^}]* in your regex as that's what you really seem to mean.

• Consider making your regexen more robust in general. Your substitution:

  $final =~ s/(\$\{$extractedVariable\}/$substituteWith/;
  [download]

  will work on data that looks like foo = ${bar}/baz but it will fail on foo = ${ bar }/baz because it isn't tolerant of whitespace.

• Rethink your style of naming variables. $extractedVariable might be a little confusing. Is that the variable name? Is that the value held by some variable somewhere? Etc. Something like $identifier is shorter, easier to read, and more descriptive. I also notice that after the line

  my $substituteThis = $extractedVariable;
  [download]

  you never bother to use the $substituteThis variable. You don't use it because you don't need to use it. Don't try to reassign variables to give them better names, just pick good names in the first place.

I have another suggestion that I didn't want to include in the above list because it is purely a matter of style and you may not even have a choice depending on coding guidelines in your environment. Consider naming your variables like $foo_bar rather than $fooBar. I strongly agree with those that claim it makes for much more readable code.

-sauoq
"My two cents aren't worth a dime.";

sauoq,

Thank you for the suggestions. I will try the regex this monday. I need to reply to a couple of your points though

Get rid of your isDefined() subroutine. All it is doing is allowing you to replace
I am not the only one who uses this function. It has great meaning somewhere else

Consider making your regexen more robust in general.
The regex I chose is exactly what I am requireing. There is to be no whitespace allowed when expanding property values

The $substituteThis and $substituteWith variables were used before I asked for help. I just hadn't pruned them out yet. I will rename $extractedVariable to $extractedVariableName

The style thing. I come from a C programming background into a Perl and then Java. My preference is the Java variety.

---

Kristofer Hoch

Si vos can lego is, vos es super erudio

A routine that turns defined $hashref->{$key} into $self->isDefined($hashref, $key) doesn't seem to have any purpose other than obfuscation to me. At least as far as I'm concerned, your code immediately became twice as clear once I substituted that for defined.

Maybe it's an interface consistence thing that makes you have this method in your class - but that doesn't mean your routine should be using it.

Makeshifts last the longest.

sub interpolateValues {
    return unless defined $INTWeb::Properties::INTERPOLATE_VALUES;

    my $self = shift;
    my $list = shift;

    my $get_value_for = sub {
        my $var = shift;
        # !!!
        # the following doesn't seem right
        # don't you need to return $list->{var}
        # if ->{Resolved}->{var} is undef?
        # that seems to be your culprit
        return $self->{Resolved}->{$var}
            if defined $list->{$var}
            or defined $self->{Resolved}->{$var};
        
        die "FATAL: $var is not a valid propery variable name\n";
    };

    while(my($key, $val) = each %$list) {
        if($val =~ s/\$\{(.*?)\}/$get_value_for->($1)/ge) {
            # ???
            # you don't seem to be storing this anywhere?
        }
        else {
            $self->{Resolved}->{$key} = $val;
            delete $list->{$key};
        }
    }
}
[download]

Makeshifts last the longest.