ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peter Reilly" <peter.kitt.rei...@gmail.com>
Subject Re: svn commit: r577308 - in /ant/core/trunk/src/main/org/apache/tools/ant/taskdefs: ProcessDestroyer.java RecorderEntry.java Redirector.java Sleep.java StreamPumper.java Tar.java Zip.java
Date Thu, 20 Sep 2007 10:59:19 GMT
Yes, It is difficult to come up with good names....

When I see wait(1000), my brain translates this it wait
for a signal, but if it does not happen in a second,
give up.

So in this case, one second as a name sounds reasonable
(or at least better than 1000).

Peter


On 9/20/07, Jan.Materne@rzf.fin-nrw.de <Jan.Materne@rzf.fin-nrw.de> wrote:
> >URL: http://svn.apache.org/viewvc?rev=577308&view=rev
> >Log: more magic numbers
> >
> >
> >Modified:
> >ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/ProcessDestroyer.
> java
> > class ProcessDestroyer implements Runnable {
> >-
> >+    private static final int TWENTY_SECONDS = 20000;
> >     private Vector processes = new Vector();
> >     // methods to register and unregister shutdown hooks
> >     private Method addShutdownHookMethod;
> >             try {
> >-                destroyProcessThread.join(20000);
> >+                destroyProcessThread.join(TWENTY_SECONDS);
>
>
> You should use the meaning of the constant as name, not its value.
>
>
> >Modified:
> >ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/Redirector.java
> > public class Redirector {
> >+    private static final int ONE_SECOND = 1000;
> >
> >     private static final String DEFAULT_ENCODING
> >         = System.getProperty("file.encoding");
> >-                wait(1000);
> >+                wait(ONE_SECOND);
>
>
> again - describe the meaning not the value
>
>
> >ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/StreamPumper.java
> (original)
> > public class StreamPumper implements Runnable {
> >
> >+    private static final int SMALL_BUFFER_SIZE = 128;
> >-    private int bufferSize = 128;
> >+    private int bufferSize = SMALL_BUFFER_SIZE;
>
>
> exactly like here. Its not HUNDRED_TWENTY_EIGHT, it is SMALL_BUFFER_SIZE
> ;)
>
>
> Jan
>
> ---------------------------------------------------------------------
> 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


Mime
View raw message