Oliver Wong said:
I'll put it this way: If later on, you decide to change ResumeData so
that it reads from a SQL Database instead of a file, you will need to make
zero changes to ResumeWriterAscii. Similarly, if you decide to change the
fileformat for the file you're reading from, or if you have ResumeData
query the user for their employment history in a fancy GUI, like some sort
of "automatic resume generating wizard".
In all cases, ResumeWriterFoo doesn't know or care where the data is
coming from.
Okay, that makes good sense to me since it means that I can get the data
from some other source entirely, like a database, without having to change
any of the other classes. By that standard my code is already avoiding
direct interaction since the getters are in ResumeData; ResumeWriterFoo
invokes those getters via an instance variable but nothing in
ResumeWriterFoo would change if the code in ResumeData changed to get the
data from a different place.
I'd have to see ResumeUtilities (or parts of it), to be sure, but
usually giving a class the name "Utilities" implies a not-so-OO design. It
usually means you have a bunch of functions floating around, not
associated with any one particular object, so you decided to gather them
together and call them a set of utilities.
That's pretty much exactly what happened
Here is the code, with package statement, imports, and comments omitted:
--------------------------------------------------------------------------------------------------------------
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.i
utputStreamWriter;
import java.io.PrintWriter;
import ca.maximal.common.utilities.DateTimeUtils;
public class ResumeUtilities {
public static final String CLASS_NAME = "ResumeUtilities";
public static void deleteFile(String fileName) {
/* Delete the old version of the output file. */
File outfile = new File(fileName);
outfile.delete();
}
public static PrintWriter openOutputFile(String fileName) {
String METHOD_NAME = "openOutputFile()";
FileOutputStream fileOutputStream = null;
OutputStreamWriter outputStreamWriter = null;
PrintWriter printWriter = null;
try {
fileOutputStream = new FileOutputStream(fileName, true);
} catch (FileNotFoundException excp) {
System.err.println(CLASS_NAME + "." + METHOD_NAME + " - Tried to
open file " + fileName + ". Error: " + excp);
excp.printStackTrace();
return printWriter;
}
outputStreamWriter = new OutputStreamWriter(fileOutputStream);
printWriter = new PrintWriter(new
BufferedWriter(outputStreamWriter), true);
return printWriter;
}
public static void closeOutputFile(PrintWriter printWriter) {
printWriter.flush();
printWriter.close();
}
public static String getGeneratedBy(String className) {
String currentDate = DateTimeUtils.getCurrentDateIso();
String currentTime = DateTimeUtils.getCurrentTime();
String generatedBy = "This file was generated by " + className + "
on " + currentDate + " at " + currentTime + ".";
return generatedBy;
}
}
--------------------------------------------------------------------------------------------------------------
Basically, these methods occurred in _almost_ every one of my file
generating classes. (Remember, _some_ of the files being generated are
resumes but some are not; the files which aren't resumes include a Javadoc
overview, an HTML page that has links to the different resume files, an HTML
page that displays the resume version of the applet, files containing CSS,
and a PDF that contains professional references who can vouch for me.)
Hmmm.....
I've wondered several times if I should have an abstract class with a name
like FileWriter that contains methods shared by _all_ of the classes which
write files, then have ResumeWriterAscii and its siblings be subclasses of
FileWriter. If I did that, FileWriter would be a logical place to put the
methods in ResumeUtilties so that they would be available to all of the
classes that write files, whether they are resumes or other files. This
version of the design also seems like a more complete design since it
recognizes _all_ of the files I'm writing, not just the resumes.
In fact, I've just made a lengthy exploration of that exact design. (I made
one wrong turn that messed me up for a while but then I reasoned my way out
of it.) I was able to make this new design work.
So, here's what I came up with.
I created a new file called FileWriter. It is essentially just the
FileUtilities class I showed you earlier in this note but I've made the
formerly static methods non-static and added an empty constructor. At the
moment, FileWriter is abstract but I don't quite see the point in leaving it
that way since it has no abstract methods in it; unless you can suggest a
good reason, I'm going to make it concrete.
Now, all of the classes that write files, regardless of whether they are
resume files or other supporting files, subclass FileWriter. Since
ResumeApplet does _not_ WRITE a resume - it displays it in the applet GUI,
ResumeApplet does _not_ subclass FileWriter.
ResumeGenerator got a bit of a facelift and now looks like this (package,
constants, and comments omitted):
--------------------------------------------------------------------------------------------------------------
public class ResumeGenerator3 implements ResumeConstants,
ResumeGeneratorConstants {
public static void main(String[] args) {
new ResumeGenerator3();
}
public ResumeGenerator3() {
ResourceBundle resumeResources =
LocalizationUtils.getResources(Locale.getDefault(), LIST_BASE);
ResumeData resumeData = new ResumeData(resumeResources);
new ResumeWriterHtml3().writeResume(OUTPUT_FILE_PATH +
RESUME_HTML_FILE, resumeData);
new ResumeWriterAscii3().writeResume(OUTPUT_FILE_PATH +
RESUME_ASCII_FILE, resumeData);
new ResumeWriterPdf3().writeResume(OUTPUT_FILE_PATH +
RESUME_PDF_FILE, resumeData);
new ResumeMainHtml3().writeFile(OUTPUT_FILE_PATH +
RESUME_MAIN_HTML_FILE);
new ResumeExampleHtml3().writeFile(OUTPUT_FILE_PATH +
RESUME_EXAMPLE_HTML_FILE);
new ResumeCssDisplay3().writeFile(OUTPUT_FILE_PATH +
CSS_DISPLAY_FILE);
new ResumeCssPrint3().writeFile(OUTPUT_FILE_PATH + CSS_PRINT_FILE);
new ResumeJavadocOverview3().writeFile(OUTPUT_FILE_PATH +
RESUME_JAVADOC_OVERVIEW_FILE);
new ResumeReferencesPdf3().writeFile(OUTPUT_FILE_PATH +
RESUME_REFERENCES_PDF_FILE);
return;
}
}
--------------------------------------------------------------------------------------------------------------
I realized that it made no sense to instantiate ResumeData more than once
since I wanted all of the resume writing classes to use the exact same data;
why pay for creation of ResumeData three times? (I will have to pay for it
one additional time, when I run ResumeApplet.) So, I simply do the work once
in ResumeGenerator: I get the ResourceBundle, then use that to get the
ResumeData. Then, I instantiate each file writing class and invoke the
method that writes that particular file, passing it the name of the output
file and the resume data if it is needed by that class.
[Side note: am I correct in understanding that
new Foo().writeFile();
instantiates class Foo, performs its no-parameter constructor, then executes
method writeFile()? I've never seen that approach before but it seems like
that is what it should do.]
The various file writing classes got modified so that they have a trivial
constructor, then do the real work in the write() method.
Here is an example of one of classes that creates a non-resume file:
--------------------------------------------------------------------------------------------------------------
public class ResumeCssPrint3 extends FileWriter implements ResumeConstants,
ResumeGeneratorConstants {
final String MY_CLASS_NAME = getClass().getName();
public ResumeCssPrint3() {
super();
}
public void writeFile(String outfile) {
deleteFile(outfile);
PrintWriter printWriter = openOutputFile(outfile);
String whiteHex = ColorConversionUtils.getHexColor(Color.WHITE);
String blackHex = ColorConversionUtils.getHexColor(Color.BLACK);
/* Write the CSS. */
printWriter.println("/* " +
ResumeUtilities.getGeneratedBy(this.MY_CLASS_NAME) + " */");
printWriter.println("/* Link in HTML to stylesheet */");
printWriter.println("/* <LINK rel=\"stylesheet\" TYPE=\"text/css\"
MEDIA=\"print\" HREF=\"" + CSS_PRINT_FILE + "\"> */");
printWriter.println("");
printWriter.println("/* " + RESUME_UNICODE_CAMELCASE + " Style Sheet
*/");
printWriter.println("");
...
closeOutputFile(printWriter);
System.out.println("The print version of the CSS for the " +
RESUME_UNICODE_LOWERCASE + " pages was written to file " + outfile + ".");
return;
}
}
--------------------------------------------------------------------------------------------------------------
And here is an example of one of the classes that creates a resume:
--------------------------------------------------------------------------------------------------------------
public class ResumeWriterAscii3 extends FileWriter implements ResumeWriter3,
ResumeConstants {
public ResumeWriterAscii3() {
super();
}
public final void writeResume(String outfile, ResumeData resumeData) {
deleteFile(outfile);
PrintWriter printWriter = openOutputFile(outfile);
StringUtils stringUtils = new StringUtils();
/* Write the candidate name and address data. */
printWriter.println(stringUtils.wrap(resumeData.getCandidateNameData().toUpperCase(),
ResumeWriterAscii3.DELIMS, ResumeWriterAscii3.LINE_WIDTH,
ResumeWriterAscii3.INDENT1, ResumeWriterAscii3.INDENT2_NORMAL,
ResumeWriterAscii3.BLANK_LINES_BEFORE_0,
ResumeWriterAscii3.BLANK_LINES_AFTER_1));
for (String candidateAddressLine :
resumeData.getCandidateAddressData()) {
printWriter.println(candidateAddressLine);
}
/* Write the job target. */
printWriter.println(stringUtils.wrap(resumeData.getJobTargetText() +
": " + resumeData.getJobTargetData(), ResumeWriterAscii3.DELIMS,
ResumeWriterAscii3.LINE_WIDTH, ResumeWriterAscii3.INDENT1,
ResumeWriterAscii3.INDENT2_NORMAL, ResumeWriterAscii3.BLANK_LINES_BEFORE_1,
ResumeWriterAscii3.BLANK_LINES_AFTER_2)); //$NON-NLS-1$
...
/* Close the output file. */
closeOutputFile(printWriter);
/* Tell the user where to find the report. */
System.out.println("The ASCII version of the " +
RESUME_UNICODE_LOWERCASE + " was written to file " + outfile + ".");
}
}
--------------------------------------------------------------------------------------------------------------
As you can see, I've gone ahead and moved all the code that used to be in
the small writeXXX methods into writeResume(). The whole class is less than
200 lines, even including the constants and comments. See below for why I
did this....
If the only class using the methods in ResumeUtilities are the
ResumeWriters, then it probably makes sense to move those methods into
ResumeWriter (and possibly making them private or protected).
As for the writeXXX, I'd leave them where they are (i.e. NOT in the
abstract super class). To do otherwise would force any future subclass of
ResumeWriter to implement those methods, without bringing much gain (since
the writeXXX are only called internally anyway).
Since you say the sections never change, then perhaps your write()
method is the same for all ResumeWriterFoo? If so, then go ahead and move
that into the super class ResumeWriter. If they differ, then leave them
where they are.
The write() method in ResumeWriterAscii and its siblings does _not_
currently have the same implementation in each class that implements it -
but it _could_ easily enough. After you expressed your discomfort with my
original formulation of the write() method in ResumeWriterAscii, I changed
the write method as follows:
-----------------------------------------------------------------------------------------------------
public final void writeResume(ResumeData resumeData) {
writeCandidateAndTarget(resumeData);
writeEmploymentHistory(resumeData);
writeFormalEducation(resumeData);
writeRecentEducation(resumeData);
writeEmployability(resumeData);
writeAddendum(resumeData);
writeOses(resumeData);
writeComputerLanguages(resumeData);
writeDatabases(resumeData);
writeWordProcessors(resumeData);
writeEditors(resumeData);
writeOtherTools(resumeData);
writeOtherExperience(resumeData);
writeImportantNote(resumeData);
}
-----------------------------------------------------------------------------------------------------
The write() method for the other ResumeWriterXXX classes are very similar
except that some have some extra methods.
But it would be very easy to simply take the code from each of the writeXXX
methods and simply move it into the write() method; then it wouldn't invoke
any other methods at all and simply write the entire resume in one long but
very simple method; there are a couple of simple loops but not a single if
or case. Frankly, I'm not sure I see any point in splitting the writing up
amongst all these little writeXXX methods aside from the fact that each
writeXXX method is very short, just a few lines long. That makes the
functionality of each writeXXX method very simple to understand.
I know there are people who instantly loathe methods that are longer than a
handful of lines no matter what code is contained within the method and who
ruthlessly break code up so that no method contains more than, say, 7 lines.
Frankly, I've never quite understood that reasoning; to me, there's nothing
wrong with a long method, even hundreds of lines long, as long as it is very
linear. But if it contains more than very small number of ifs or branches, I
am inclined to break it into smaller pieces. With regards to my write()
methods though, they would be longish but very very linear so I don't see
much harm in doing all the writing directly within the write() method of
each ResumeWriterXXX class.
Since I was making the other changes that I mentioned earlier in the note
anyway, I went ahead and moved the writeXXX code into the write() method in
each of the ResumeWriterXXX classes _except_ ResumeWriterPdf; that one had
its writeXXX methods left alone because there was somewhat more code
involved in writing the PDF so it made more sense to retain the existing
small writeXXX methods.
Now, I would be curious to hear your reaction to my new design, particularly
the new class FileWriter and how it is used. Is it a step forward or
backward? Is it a reasonable design?
Frankly, I'm a bit uneasy about FileWriter since I'm not quite sure what it
_is_. I can't really say that it is a class that can create and write a file
since there are no write methods in it, just ones for deleting, opening, and
closing files. All it can really do is create an _empty_ file. So maybe it
needs some kind of (abstract?) write methods in it that can be implemented
in its subclasses - or _not_ implemented in the ResumeWriterXXX classes
since they already implement write() (actually, I've renamed it
writeResume() as you may have noticed in my code excerpts). I'm also not
sure that the ResumeWriterXXX classes subclass it. Then again, it all
_works_ very well so maybe it is not so bad. And I can't think of anything
else that particularly bothers me about the code now.
I'd be interested in your opinion - as well as anyone else who is still
watching this thread.
Rhino
