>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.

The Vector was only added because the comma delimted list in the properties 
file gets translated to a vector.

>I actually dislike  modifying passed-in parameters

Why?  

>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?

Err, you don't use JAVA_HOME?  I'm guessing you're not a *nix user ?  

The reason I want them in Velocity, is file.resource.loader.path only accepts a 
full path.  With several developers working on a project its a pain to have 
explain how to update this for everyone who wants to run the project.


I agree the ExtendedProperties class is what needs patching, I don't know how 
receptive they will be though.


Thanks,
Charlei




-----Original Message-----
From: Christopher Schultz [mailto:[EMAIL PROTECTED]
Sent: Tue 5/6/2008 5:04 PM
To: Velocity Developers List
Subject: Re: Expanding Environmental variables patch
 
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?

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.

-chris


Reply via email to