You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by ke...@apache.org on 2006/04/14 03:24:04 UTC
svn commit: r393996 -
/ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Author: kevj
Date: Thu Apr 13 18:24:01 2006
New Revision: 393996
URL: http://svn.apache.org/viewcvs?rev=393996&view=rev
Log:
slight tweak while -> for (reduce scope of variable to just loop)
Modified:
ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Modified: ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
URL: http://svn.apache.org/viewcvs/ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java?rev=393996&r1=393995&r2=393996&view=diff
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java Thu Apr 13 18:24:01 2006
@@ -114,10 +114,9 @@
*/
private void addPath(String path, boolean getJars, List libPathURLs)
throws MalformedURLException {
- StringTokenizer myTokenizer
- = new StringTokenizer(path, File.pathSeparator);
- while (myTokenizer.hasMoreElements()) {
- String elementName = myTokenizer.nextToken();
+ for (StringTokenizer tzr = new StringTokenizer(path, File.pathSeparator);
+ tzr.hasMoreElements();) {
+ String elementName = tzr.nextToken();
File element = new File(elementName);
if (elementName.indexOf("%") != -1 && !element.exists()) {
continue;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Kevin Jackson <fo...@gmail.com>.
On 4/17/06, Stefan Bodewig <bo...@apache.org> wrote:
> On Fri, 14 Apr 2006, Martijn Kruithof <jk...@apache.org> wrote:
>
> > What's the position of other committers on this?
>
> Joshua Bloch uses the for loop construct in "Effective Java", I think
> this is where I saw it first. Back then I decided to not pick up his
> advice but keep my old while loop style for iterators.
>
I recall seeing it first in Effective Java too - it made sense to me
then and I now use that style of looping for all
iterators/enumerations and tokenizers in my own code - seems it isn't
really that common an idiom however
Anyway, interesting to see how other Java developers view things that
I've always thought of as standard practice - certainly made me think
- which is always good
Kev
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 -
/ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Stefan Bodewig <bo...@apache.org>.
On Fri, 14 Apr 2006, Martijn Kruithof <jk...@apache.org> wrote:
> What's the position of other committers on this?
Joshua Bloch uses the for loop construct in "Effective Java", I think
this is where I saw it first. Back then I decided to not pick up his
advice but keep my old while loop style for iterators.
Kev and Dominique are certainly correct when they talk about scoping -
which is the primary benefit of the for loop idiom. But unless I have
very long methods with multiple loops I prefer to stick to the while
idiom that - at least I feel - is more readable. If I have such long
methods I try to split them anyway 8-)
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Dominique Devienne <dd...@gmail.com>.
> ok, I think the consensus is that I shouldn't have committed this
> change ie -1 to this change from while -> for.
I'm also a proponent of using for loops rather than while loops for
Iterators. I picked up the idiom reading "The Java Programming
Language", and I like the added scope it provides, which is useful
since I tend to use just i (or j / k for nested loops) for the
iterator variable.
In this particular instance, it doesn't help much though, as the var
has an explicit name, the method is short, and the for loop itself is
broken down on 2 lines.
OTOH, I don't like Antoine's for loop ;-) I haven't tried, but I
wouldn't have thought it compiled either ;-))) -DD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Kevin Jackson <fo...@gmail.com>.
ok, I think the consensus is that I shouldn't have committed this
change ie -1 to this change from while -> for.
I'll change it back to the while loop on Monday when I get to work.
Thanks
Kev
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Martijn Kruithof <jk...@apache.org>.
Antoine Levy-Lambert wrote:
>I also think that the code was more readable before. The variable
>myTokenizer or tzr being a variable of a private function goes quickly
>out of scope anyway.
>
>Something like this would alleviate the remark of Martijn concerning
>where the incrementation takes place but does not seem to be proper java
>syntax.
>for (StringTokenizer tzr = new StringTokenizer(path, File.pathSeparator)
>&& String elementName = tzr.nextToken();
>tzr.hasMoreElements();elementName = tzr.nextToken()) {
>
>
To be honest I don't really like this for loop either, the && in the
initializer of the loop doesn't make it more readable.
My general feeling is that "funny" constructs used to reduce scope by
just defining something inside the loop control statement is that in
that case a method should be used to reduce the lifetime and scope of a
variable.
In the case of the changed loop, the scope of the variable was not
reduced by any statement. (Method terminates directly after the last
statement of the loop.)
Generally I am more in favour of creating an extra method, than to
introduce unclear constructs to reduce scope.
(And in this case the method is already limited to the action in the loop.)
Martijn
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Antoine Levy-Lambert <an...@gmx.de>.
Martijn Kruithof wrote:
> Hi,
>
> I think the while construct is cleaner and more readable, and do not
> see any real benefit of this code change.
> Especially against using the for loop is that when using the for loop
> the step of the loop should take place in the third part of the for,
> and not somewhere inside the for loop.
>
> What's the position of other committers on this?
>
I also think that the code was more readable before. The variable
myTokenizer or tzr being a variable of a private function goes quickly
out of scope anyway.
Something like this would alleviate the remark of Martijn concerning
where the incrementation takes place but does not seem to be proper java
syntax.
for (StringTokenizer tzr = new StringTokenizer(path, File.pathSeparator)
&& String elementName = tzr.nextToken();
tzr.hasMoreElements();elementName = tzr.nextToken()) {
> Martijn
>
>
Antoine
> kevj@apache.org wrote:
>
>> Author: kevj
>> Date: Thu Apr 13 18:24:01 2006
>> New Revision: 393996
>>
>> URL: http://svn.apache.org/viewcvs?rev=393996&view=rev
>> Log:
>> slight tweak while -> for (reduce scope of variable to just loop)
>>
>> Modified:
>> ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
>>
>> Modified:
>> ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
>> URL:
>> http://svn.apache.org/viewcvs/ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java?rev=393996&r1=393995&r2=393996&view=diff
>>
>> ==============================================================================
>>
>> --- ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
>> (original)
>> +++ ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
>> Thu Apr 13 18:24:01 2006
>> @@ -114,10 +114,9 @@
>> */
>> private void addPath(String path, boolean getJars, List libPathURLs)
>> throws MalformedURLException {
>> - StringTokenizer myTokenizer
>> - = new StringTokenizer(path, File.pathSeparator);
>> - while (myTokenizer.hasMoreElements()) {
>> - String elementName = myTokenizer.nextToken();
>> + for (StringTokenizer tzr = new StringTokenizer(path,
>> File.pathSeparator); + tzr.hasMoreElements();) {
>> + String elementName = tzr.nextToken();
>> File element = new File(elementName);
>> if (elementName.indexOf("%") != -1 && !element.exists()) {
>> continue;
>>
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Kev Jackson <ke...@it.fts-vn.com>.
Martijn Kruithof wrote:
> Hi,
>
> I think the while construct is cleaner and more readable, and do not
> see any real benefit of this code change.
> Especially against using the for loop is that when using the for loop
> the step of the loop should take place in the third part of the for,
> and not somewhere inside the for loop.
>
My reason for the change was that with the while construct, the scope of
the tokenizer variable is larger than is necessary - ie it is only used
within the loop, yet it is declared outside the loop.
The idiom I was using when making this change is similar to
for (Iterator i = list.iterator(); i.hasNext();) {
Stuff s = (Stuff)i.next();
...
}
Which is fairly common usage - ignoring the last element of the for
> What's the position of other committers on this?
>
I'd be interested to hear other opinions too, after all I only changed
the code as I (perhaps mistakenly) thought that having a larger than
necessary scope is generally a bad thing
> Martijn
Thanks for the comments
Kev
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r393996 - /ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
Posted by Martijn Kruithof <jk...@apache.org>.
Hi,
I think the while construct is cleaner and more readable, and do not see
any real benefit of this code change.
Especially against using the for loop is that when using the for loop
the step of the loop should take place in the third part of the for, and
not somewhere inside the for loop.
What's the position of other committers on this?
Martijn
kevj@apache.org wrote:
>Author: kevj
>Date: Thu Apr 13 18:24:01 2006
>New Revision: 393996
>
>URL: http://svn.apache.org/viewcvs?rev=393996&view=rev
>Log:
>slight tweak while -> for (reduce scope of variable to just loop)
>
>Modified:
> ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
>
>Modified: ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java
>URL: http://svn.apache.org/viewcvs/ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java?rev=393996&r1=393995&r2=393996&view=diff
>==============================================================================
>--- ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java (original)
>+++ ant/core/trunk/src/main/org/apache/tools/ant/launch/Launcher.java Thu Apr 13 18:24:01 2006
>@@ -114,10 +114,9 @@
> */
> private void addPath(String path, boolean getJars, List libPathURLs)
> throws MalformedURLException {
>- StringTokenizer myTokenizer
>- = new StringTokenizer(path, File.pathSeparator);
>- while (myTokenizer.hasMoreElements()) {
>- String elementName = myTokenizer.nextToken();
>+ for (StringTokenizer tzr = new StringTokenizer(path, File.pathSeparator);
>+ tzr.hasMoreElements();) {
>+ String elementName = tzr.nextToken();
> File element = new File(elementName);
> if (elementName.indexOf("%") != -1 && !element.exists()) {
> continue;
>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
>For additional commands, e-mail: dev-help@ant.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org