Refactor Builder constructor for readability

Squareys · Squareys · commit c2a33d37cc3f · 2015-03-10T12:34:25.000+01:00
There are now two constructors which clearly differentiate between creating
a project for a File and creating a project for a Reader.
This makes the calling code easier to read.

To avoid code duplication, two small private methods were added to Builder.
diff --git a/src/main/java/org/scijava/plugins/scripting/java/JavaEngine.java b/src/main/java/org/scijava/plugins/scripting/java/JavaEngine.java
@@ -182,10 +182,21 @@ public Class<?> compile(String script) throws ScriptException {
 
 		final Writer writer = getContext().getErrorWriter();
 		try {
-			// script may be null, but then we cannot create a StringReader for it,
-			// therefore null is passed if script is null.
-			final Reader reader = (script == null) ? null : new StringReader(script);
-			final Builder builder = new Builder(file, reader, writer);
+
+			final Builder builder;
+
+			if (file != null && file.exists()) {
+				// if the filename set in engine scope bindings is valid,
+				// ignore the given script and use that file instead.
+				builder = new Builder(file, writer);
+			}
+			else {
+				// script may be null, but then we cannot create a StringReader for it,
+				// therefore null is passed if script is null.
+				final Reader reader =
+					(script == null) ? null : new StringReader(script);
+				builder = new Builder(reader, writer);
+			}
 			final MavenProject project = builder.project;
 			String mainClass = builder.mainClass;
 
@@ -277,7 +288,7 @@ public void compile(final File file, final Writer errorWriter) {
 		try {
 			final Writer writer =
 				(errorWriter == null) ? getContext().getErrorWriter() : errorWriter;
-			final Builder builder = new Builder(file, null, writer);
+			final Builder builder = new Builder(file, writer);
 			try {
 				builder.project.build();
 			}
@@ -302,7 +313,7 @@ public void makeJar(final File file, final boolean includeSources,
 		final File output, final Writer errorWriter)
 	{
 		try {
-			final Builder builder = new Builder(file, null, errorWriter);
+			final Builder builder = new Builder(file, errorWriter);
 			try {
 				builder.project.build(true, true, includeSources);
 				final File target = builder.project.getTarget();
@@ -346,6 +357,7 @@ private void printOrThrow(Throwable t, Writer errorWriter) {
 	 * A wrapper around a (possibly only temporary) project.
 	 * 
 	 * @author Johannes Schindelin
+	 * @author Jonathan Hale
 	 */
 	private class Builder {
 
@@ -355,11 +367,11 @@ private class Builder {
 		private MavenProject project;
 
 		/**
-		 * Constructs a wrapper around a possibly temporary project.
+		 * Constructs a wrapper around a possibly project for a source or maven
+		 * project file.
 		 * 
 		 * @param file the {@code .java} file to build (or null, if {@code reader}
 		 *          is set).
-		 * @param reader provides the Java source if {@code file} is {@code null}
 		 * @param errorWriter where to write the error output.
 		 * @throws ScriptException
 		 * @throws IOException
@@ -369,48 +381,91 @@ private class Builder {
 		 * @throws TransformerException
 		 * @throws TransformerFactoryConfigurationError
 		 */
-		private Builder(final File file, final Reader reader,
-			final Writer errorWriter) throws ScriptException, IOException,
-			ParserConfigurationException, SAXException,
-			TransformerConfigurationException, TransformerException,
+		private Builder(final File file, final Writer errorWriter)
+			throws ScriptException, IOException, ParserConfigurationException,
+			SAXException, TransformerConfigurationException, TransformerException,
 			TransformerFactoryConfigurationError
 		{
-			if (errorWriter == null) {
-				err = null;
-			}
-			else {
-				err = new PrintStream(new LineOutputStream() {
+			err = createErrorPrintStream(errorWriter);
 
-					@Override
-					public void println(final String line) throws IOException {
-						errorWriter.append(line).append('\n');
-					}
+			BuildEnvironment env = createBuildEnvironment();
 
-				});
+			// will throw IOException if file does not exist.
+			temporaryDirectory = null;
+			if (file.getName().equals("pom.xml")) {
+				project = env.parse(file, null);
 			}
+			else {
+				mainClass = getFullClassName(file);
+				project = getMavenProject(env, file, mainClass);
+			}
+		}
 
-			boolean verbose = "true".equals(get("verbose"));
-			boolean debug = "true".equals(get("debug"));
-			BuildEnvironment env = new BuildEnvironment(err, true, verbose, debug);
+		/**
+		 * Constructs a wrapper around a possibly temporary project for source code
+		 * generated by a Reader.
+		 * 
+		 * @param reader provides the Java source if {@code file} is {@code null}
+		 * @param errorWriter where to write the error output.
+		 * @throws ScriptException
+		 * @throws IOException
+		 * @throws ParserConfigurationException
+		 * @throws SAXException
+		 * @throws TransformerConfigurationException
+		 * @throws TransformerException
+		 * @throws TransformerFactoryConfigurationError
+		 */
+		private Builder(final Reader reader, final Writer errorWriter)
+			throws ScriptException, IOException, ParserConfigurationException,
+			SAXException, TransformerConfigurationException, TransformerException,
+			TransformerFactoryConfigurationError
+		{
+			err = createErrorPrintStream(errorWriter);
+
+			BuildEnvironment env = createBuildEnvironment();
 
-			if (file == null || !file.exists()) try {
+			try {
 				project = writeTemporaryProject(env, reader);
 				temporaryDirectory = project.getDirectory();
 				mainClass = project.getMainClass();
 			}
 			catch (Exception e) {
 				throw new ScriptException(e);
 			}
-			else {
-				temporaryDirectory = null;
-				if (file.getName().equals("pom.xml")) {
-					project = env.parse(file, null);
-				}
-				else {
-					mainClass = getFullClassName(file);
-					project = getMavenProject(env, file, mainClass);
-				}
+		}
+
+		/**
+		 * Create a {@link PrintStream} from an error {@link Writer}.
+		 * 
+		 * @param errorWriter the {@link Writer} to write errors to.
+		 * @return a {@link PrintStream} which writes to errorWriter.
+		 */
+		private PrintStream createErrorPrintStream(final Writer errorWriter) {
+			if (errorWriter == null) {
+				return null;
 			}
+
+			// create a PrintStream which redirects output to errorWriter
+			return new PrintStream(new LineOutputStream() {
+
+				@Override
+				public void println(final String line) throws IOException {
+					errorWriter.append(line).append('\n');
+				}
+
+			});
+		}
+
+		/**
+		 * Create a {@link BuildEnvironment} for current engine scope bindings
+		 * context.
+		 * 
+		 * @return the created {@link BuildEnvironment}.
+		 */
+		private BuildEnvironment createBuildEnvironment() {
+			boolean verbose = "true".equals(get("verbose"));
+			boolean debug = "true".equals(get("debug"));
+			return new BuildEnvironment(err, true, verbose, debug);
 		}
 
 		/**
