- changed a for loop - added a new private method to check for non null params - used new method in the appropriate places
Unit test for two changes
Kev
Index: AbstractCvsTask.java
===================================================================
RCS file:
/home/cvspublic/ant/src/main/org/apache/tools/ant/taskdefs/AbstractCvsTask.java,v
retrieving revision 1.35
diff -u -r1.35 AbstractCvsTask.java
--- AbstractCvsTask.java 17 Jul 2004 15:10:11 -0000 1.35
+++ AbstractCvsTask.java 28 Dec 2004 11:20:36 -0000
@@ -390,7 +390,7 @@
}
try {
- for (int i = 0; i < vecCommandlines.size(); i++) {
+ for (int i = 0, size = vecCommandlines.size(); i < size ; i++) {
this.runCommand((Commandline) vecCommandlines.elementAt(i));
}
} finally {
@@ -440,21 +440,27 @@
return stringBuffer.toString();
}
+ //added because same functionality is checked 4 times in this source file
+ //another method call, so maybe a hit in performance
+ private static boolean isNonNullParam(final String paramToCheck) {
+ boolean res = false;
+ if ((paramToCheck != null) && (paramToCheck.trim().length() > 0)) {
+ res = true; //real param
+ }
+ return res;
+ }
+
/**
* The CVSROOT variable.
- *
* @param root the CVSROOT variable
*/
- public void setCvsRoot(String root) {
-
+ public void setCvsRoot(final String root) {
// Check if not real cvsroot => set it to null
- if (root != null) {
- if (root.trim().equals("")) {
- root = null;
- }
+ if (isNonNullParam(root)) {
+ this.cvsRoot = root;
+ } else {
+ this.cvsRoot = null;
}
-
- this.cvsRoot = root;
}
/**
@@ -462,24 +468,20 @@
* @return CVSROOT
*/
public String getCvsRoot() {
-
return this.cvsRoot;
}
/**
* The CVS_RSH variable.
- *
* @param rsh the CVS_RSH variable
*/
- public void setCvsRsh(String rsh) {
- // Check if not real cvsrsh => set it to null
- if (rsh != null) {
- if (rsh.trim().equals("")) {
- rsh = null;
- }
+ public void setCvsRsh(final String rsh) {
+ // Check if not real cvsroot => set it to null
+ if (isNonNullParam(rsh)) {
+ this.cvsRsh = rsh;
+ } else {
+ this.cvsRsh = null;
}
-
- this.cvsRsh = rsh;
}
/**
@@ -487,14 +489,12 @@
* @return the CVS_RSH variable
*/
public String getCvsRsh() {
-
return this.cvsRsh;
}
/**
* Port used by CVS to communicate with the server.
- *
- * @param port port of CVS
+ * @param port port of CVS
*/
public void setPort(int port) {
this.port = port;
@@ -505,7 +505,6 @@
* @return the port of CVS
*/
public int getPort() {
-
return this.port;
}
@@ -542,7 +541,6 @@
/**
* get the file where the checked out files should be placed
- *
* @return directory where the checked out files should be placed
*/
public File getDest() {
@@ -552,7 +550,6 @@
/**
* The package/module to operate upon.
- *
* @param p package or module to operate upon
*/
public void setPackage(String p) {
@@ -561,11 +558,9 @@
/**
* access the package or module to operate upon
- *
* @return package/module
*/
public String getPackage() {
-
return this.cvsPackage;
}
/**
@@ -582,8 +577,8 @@
* @param p tag
*/
public void setTag(String p) {
- // Check if not real tag => set it to null
- if (p != null && p.trim().length() > 0) {
+ //If not a real tag (null/"") don't add to commandLine
+ if (isNonNullParam(p)) {
tag = p;
addCommandArgument("-r" + p);
}
@@ -619,7 +614,8 @@
* see man cvs
*/
public void setDate(String p) {
- if (p != null && p.trim().length() > 0) {
+ //don't add if null/""
+ if (isNonNullParam(p)) {
addCommandArgument("-D");
addCommandArgument(p);
}
Index: AbstractCvsTaskTest.java
===================================================================
RCS file: AbstractCvsTaskTest.java
diff -N AbstractCvsTaskTest.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ AbstractCvsTaskTest.java 1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,109 @@
+/*
+ * Created on 28-Dec-2004
+ */
+package org.apache.tools.ant.taskdefs;
+
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+
+/**
+ * @author it-kevin
+ *
+ */
+public class AbstractCvsTaskTest extends TestCase {
+
+ private String cvsRoot;
+ private String rsh;
+ private Cvs cvs = new Cvs();
+
+ /*
+ * @see TestCase#setUp()
+ */
+ protected void setUp() throws Exception {
+ super.setUp();
+ }
+
+ /*
+ * @see TestCase#tearDown()
+ */
+ protected void tearDown() throws Exception {
+ super.tearDown();
+ }
+
+ /**
+ * Constructor for AbstractCvsTaskTest.
+ * @param arg0
+ */
+ public AbstractCvsTaskTest(String arg0) {
+ super(arg0);
+ }
+
+ public static Test suite() {
+ TestSuite suite = new TestSuite();
+ suite.addTest(new AbstractCvsTaskTest("testSetCvsRoot"));
+ suite.addTest(new AbstractCvsTaskTest("testSetCvsRsh"));
+ return suite;
+ }
+
+ public void setCvsRoot(final String root) {
+
+ // Check if not real cvsroot => set it to null
+ if ((root != null) && (!root.trim().equals(""))) {
+ this.cvsRoot = root;
+ } else {
+ this.cvsRoot = null;
+ }
+ }
+
+ public void setCvsRsh(final String rsh) {
+
+ // Check if not real cvsroot => set it to null
+ if ((rsh != null) && (!rsh.trim().equals(""))) {
+ this.rsh = rsh;
+ } else {
+ this.rsh = null;
+ }
+ }
+
+ public void testSetCvsRoot() {
+ try {
+ //null
+ this.setCvsRoot(null);
+ cvs.setCvsRoot(null);
+ assertEquals(cvsRoot, cvs.getCvsRoot());
+ //"" empty string
+ this.setCvsRoot("");
+ cvs.setCvsRoot("");
+ assertEquals(cvsRoot, cvs.getCvsRoot());
+ //any other string use ant/apache cvs
+ this.setCvsRoot("/home/cvspublic");
+ cvs.setCvsRoot("/home/cvspublic");
+ assertEquals(cvsRoot, cvs.getCvsRoot());
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail();
+ }
+ }
+
+ public void testSetCvsRsh() {
+ try {
+ //null
+ this.setCvsRsh(null);
+ cvs.setCvsRsh(null);
+ assertEquals(rsh, cvs.getCvsRsh());
+ //"" empty string
+ this.setCvsRsh("");
+ cvs.setCvsRsh("");
+ assertEquals(rsh, cvs.getCvsRsh());
+ //any other string use ant/apache cvs
+ this.setCvsRsh("/home/cvspublic");
+ cvs.setCvsRsh("/home/cvspublic");
+ assertEquals(rsh, cvs.getCvsRsh());
+ } catch (Exception e) {
+ e.printStackTrace();
+ fail();
+ }
+ }
+
+}--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
