Re: Expanding Environmental variables patch

[Christopher Schultz]

Charlei,

csanders wrote:

Attached is the patch, adding three new functions, expandEnvironmentalVariables( ExtendedProperties ), expandEnvironmentalVariables(Vector ),

Any reason not to accept any Collection rather than limiting this to just a Vector? Forgive me if there are API implications coming from ExtendedProperties.

+               Vector newVector = new Vector();

This should really be:

Vectory newVector = new Vector(stringVector.size());
(or an appropriate Collection instance)

Better yet, if you are using Vectors, why not use Vector.set() instead of creating a completely new Vector every time? I actually dislike modifying passed-in parameters, but your expandEnvironmentalVariables(ExtendedProperties) method does that already, so what does it matter if the methods that it calls do the same?

The expansion should really take place in ExtendedProperties, but this might work in the interim .

Then why not patch ExtendedProperties instead?





-chris

I'm a little curious why this capability needs to be added to Velocity in the first place. Does anybody really use environment variables anymore, anyway? If anything, this should be implemented as a subclass of ExtendedProperties and handled there. There is no need to modify any Velocity code to get this to work.

signature.asc
Description: OpenPGP digital signature