Writing flexible code requires you to learn a collection to tricks (patterns). If you follow this advice, your programs will live much longer lives.
These patterns are not always the best approach at first. Sometimes you need to code quickly and then, if the code lives a few "generations", then you can refactor it to make it more flexible.
Very often your program must be run on multiple machines by multiple people. This may include running on multiple OS types. Certain values may need to change in order for your application to work. For example, often where you store data on the disk is different or who should receive email when the software causes errors (you don't want errors going to all developers, just the one testing the software).
The simplest thing to do is to create a property file full of values like this:
error.mail = parrt@jguru.com data.dir = /var/data/webmail developer = parrt
Each developer would have a different file.
Then, you can write software to ask for these values. Your application should be sensitive to these values.
// Looks for this file in CLASSPATH
public static final String WEBMAIL_PROPS = "webmail.properties";
public String getClientProperty(String name) {
InputStream in = ClassLoader.getSystemResourceAsStream(WEBMAIL_PROPS);
EnhancedProperties props = new EnhancedProperties();
props.load(inStream);
String prop = props.getProperty(name);
return prop;
}
You can even make your application load a property file based upon the host name.
String hostname = InetAddress.getLocalHost().getHostName(); File f = new File( PROPERTY_FILE_REPOSITORY, hostname+".webmail.properties" ); ...
In this case, a file called hostname.webmail.properties is loaded, automatically taking into account what machine the app will run on.
Checking for logged in state is common to most or all pages. Pull it up to a common superclass of all pages called Page.
Change this:
class ForumViewPage {
public void body() {
if ( !loggedIn ) {
// redirect to login page
response.sendRedirect("/login");
return;
}
// logged in if they reach here!
UserContext ux = session.getValue("usercontext");
int UID = ux.getPersonID();
Person person = PersonManager.getPerson(UID);
String name = person.getName();
out.println("Hi "+name);
}
}
to this:
public abstract class Page {
public void generate() {
if ( !loggedIn ) {
// redirect to login page
response.sendRedirect("/login");
return;
}
body();
}
public abstract void body();
}
class ForumViewPage extends Page {
public void body() {
...
// you know you are logged in
UserContext ux = session.getValue("usercontext");
}
}
Now you have a single place to change common functionality for all pages such as this "logged in state" test.
What is the problem with using a bunch of static methods grouped into a class like the following?
Person p = DatabaseManager.getPerson(376);
The problem is that you have to change every one of these references if you need to swap out the DatabaseManager. Instead, create an interface that lets you refer to multiple implementations (polymorphism at its finest!):
...
DBInterface db = ServiceManager.getSomeDBInterfaceImplementation();
Person p = db.getPerson(376);
...
class DatabaseManager implements DBInterface {
public Person getPerson(int id) {x}
}
class RemotedatabaseManager implements DBInterface {
public Person getPerson(int id) {do x on another machine}
}
class ServiceManager {
public static DBInterface getSomeDBInterfaceImplementation() {
if ( local ) {
return new DatabaseManager();
}
else if ( remote ) {
return new RemoteDatabaseManager();
}
}
}
As another example, consider building a visualization tool where you want both Swing and PostScript output. Define the generic concept of a graphics output engine:
public interface GraphicsEngine {
public void drawLine(int x1, int y1, int x2, int y2);
public void drawText(String txt, int x1, int y1);
...
}
and then make specific implementations:
public class SwingEngine implements GraphicsEngine {
public void drawLine(int x1, int y1, int x2, int y2) {
System.out.println("swing: drawLine");
// Graphics2D....
}
public void drawText(String txt, int x1, int y1) {
}
}
and
public class PSEngine implements GraphicsEngine {
public void drawLine(int x1, int y1, int x2, int y2) {
System.out.println("PS: drawLine");
}
public void drawText(String txt, int x1, int y1) {
}
}
Your program can generate output using a generic engine object
drawSomething(GraphicsEngine engine) {
engine.drawLine(...);
engine.drawText(...);
}
but the main program can indicate which actual implementation:
SwingEngine swing = new SwingEngine(); PSEngine ps = new PSEngine(); drawSomething(swing); drawSomething(ps);
The important thing to note is the drawSomething does not know what actual implementation is being used. Late binding determines which drawLine etc... to actually call at runtime.
Note that I've since changed my mind so that I don't always use accessors. Intellij and other IDEs know how to refactor obj.var into obj.getVar() when necessary. My new rules are essentially that booleans and arrays and any data structure are used via accessors, but simple pointers to other objects or variables are used directly. This all assumes you are using Intellij or similar that knows how to change direct field access to accessor methods via a refactoring function in case the situation changes.
Imagine you have a List like this:
class List {
public int n=0;
public Object get(int i) {...}
public Object add(Object value) {...}
}
You are tempted to access field n directly like this:
public static void main(String[] args) {
List users = new List();
users.add("Ter");
...
int size = users.n;
}
This is easy to type and very fast. The problem is that you may change the implementation of the list and then n must be computed. Ooops...now you have to go change all of your List.n references to getSize() references.
It is better to always use the accessor method for flexibility.
class List {
int n=0;
public int getSize() {...}
...
}
The on-the-fly compiler is usually smart enough to "inline" these method calls so you don't lose anything in speed, but gain all the flexibility.
I see programs that use literals directly in code such as:
int[] a = new int[100];
There are two problems with this:
Another place where I see literals where you can do better: String lengths. Instead of doing this:
// get file name from "GET file" HTTP line String file = line.substring(4, line.length());
Consider the following:
String file = line.substring("GET ".length(), line.length());
This way it's clear (particularly if somebody doesn't include the comment above).
Even better, is when you factor out your String literals. Here, "GET" is unlikely to change as billions of browsers around the world use that command. In other cases though you may want to factor out strings in case things change and you have multiple refs to the string.
public static final String SERVER_SIDE_JAVA_SIGNAL = "/java";
...
if ( file.startsWith(SERVER_SIDE_JAVA_SIGNAL) ) {
...
}
Originally jGuru code for menus was specific to our implementation rather than a table (or config property). E.g.,
public void displayMenu() {
// draw Home
// draw FAQ
...
}
rather than a table and a loop for displayMenu(). The following is must more flexible and easy to change:
public static final String[] menuItems = { "Home", "FAQ", ...}
...
public void displayMenu(String[] menuItems) {
// for each item in menuItems, display item
}
Or you can use a property or config file to specify the data. Properties are good for fontsize and erroremail etc... as well.
When building your web pages with StringTemplate, you'll see that keeping that HTML safely squirreled away in the templates and out of your code is great! Those templates can be changed without touching code and can even be updated on a live running server.
Consider the following code that prints a list of files like your HTTP server project:
String[] filelist = dir.list();
p.println("<html><title>Directory: " + dir +"</title>");
p.println("<body>");
for (int i = 0; i < filelist.length; i++) {
File f = new File(curdir + "/" + filelist[i]);
p.println("<b><font face=Arial>"+filelist[i]+"</font></b><br>");
}
p.println("</body></html>");
It is better to use a template and a code snippet:
// the template would probably be stored in a template file
StringTemplate dirST = new StringTemplate(
"<html><title>$title$</title>\n"+
"$items:{<b><font face=Arial>$attr$</font></b><br>}$
"</body></html>"
);
String[] filelist = dir.list();
dirST.setAttribute("title", "Directory: "+dir);
dirST.setAttribute("items", filelist)
p.println(dirST);
Here, the display data and code are completely isolated. Note that the template for displaying the list could be reused for any list of "stuff" page.
You can read more about it: Enforcing Model-View Separation in Template Engines.
Static (class) variables are shared among all instances of a class.
This means that in a multiple threaded environment you could end up with race conditions accessing those static variables. With multiple instances running around, each thread could operate independently on its own object.
As an example, consider a similar situation where web servers make only one instance of a Servlet. There are multiple threads (corresponding to multiple page requests) all executing on that shared object with possibility of race conditions on static variables and even instance variables. That is why I have you create a new instance of a Page object for every page request.
Usually static variables make that class a singleton class, only one instance is useful. For example, consider a database service that provides access to object stored in a database. If you have static variables, then you can likely only use one instance of the service. This would preclude you from having multiple applications running in the same Java VM. jGuru's server actually runs both jGuru and peerscope.com simultaneously in the same VM, sharing some of the code accessing data and so on.
In the old days, my parser generator generated parsers in C with lots of global variables like this:
int k = 2;
int lookahead[k];
void parseMain() {
if ( lookahead[0]==BEGIN ) {
...
}
Clearly you can only run one parser per executable here as there is only one lookahead buffer and value of k. The equivalent would be coded as follows in Java:
class MyParser {
static int k = 2;
static int lookahead[k];
static void parseMain() {
if ( lookahead[0]==BEGIN ) {
...
}
}
I should have used a struct in C to hold the values and made the parseMain function sensitive to which parser "object" to use:
struct MyParser {
int k = 2;
int lookahead[k];
};
void parseMain(struct MyParser *parser) {
if ( parser->lookahead[0]==BEGIN ) {
...
}
Now you can malloc as many parser objects as you want and the functions will happily operate on any of them.
In Java, you would see this:
class MyParser {
int k = 2;
int lookahead[k];
void parseMain() {
if ( lookahead[0]==BEGIN ) {
...
}
}
Generally speaking you should use static methods (class methods) in two situations:
When writing methods, use the most general types and the most widely-used types possible. This does two things:
If you are using a list for something, always use the List interface as the type rather than the specific implementation class like ArrayList:
public void addUser(List list, User u) {
list.add(u);
}
This code works regardless of the implementation of the list. You may pass in either an ArrayList or a LinkedList:
List a = new ArrayList(); List b = new LinkedList(); User u = ...; addUser(a, u); addUser(b, u);
As an example, I saw a student use a string template like object for generating web pages. Rather than return a String via toString the student passed in an HttpServletResponse object and had a method for writing the template out.
The first improvement would be to pass in an OutputStream rather than the response so you can test the object and its methods more easily from a test harness (you don't need a webserver). This is good from a design perspective also since you have decoupled the template from a webserver environment.
Next, you can make the object even more useful by not assuming it will print to an output stream at all and might be used in combination (embedding etc...) with other streams.
Don't tie your code to specific types or data models unless they clearly are only useful in that situation.
When your program finds a problem, spitting a message to standard error directly with System.err.println is ok, but not very flexible. For example, you might have 1000 of these print statements in a library you have built. Then, you want to make your library work from GUI applications. Messages for GUIs should go to a window not the standard out.
A good step is to make a single static method called error or something that you call from wherever:
class DatabaseManager {
public void insert(...) {
...
if ( ... ) {
Utils.error("can't insert record");
}
...
}
}
Now, you have a single point of change and the person wanting your library for use in a GUI app, can easily this code; errors will be redirected to a window.
The problem is that you might not want to provide source code with your library, just the binaries. At the very least, changing library code can be scary. There is a more flexible way to redirect errors without people having to touch your library: write errors to a special error object and have people specify which error object to use.
For example, StringTemplate allows you to create and pass in a StringTemplateErrorListener object:
public interface StringTemplateErrorListener {
public void error(String msg, Exception e);
public void warning(String msg);
public void debug(String msg);
}
The default listener looks like this:
public static StringTemplateErrorListener DEFAULT_ERROR_LISTENER =
new StringTemplateErrorListener() {
public void error(String s, Exception e) {
System.err.println(s);
if ( e!=null ) {
e.printStackTrace(System.err);
}
}
public void warning(String s) {
System.out.println(s);
}
public void debug(String s) {
System.out.println(s);
}
}
On a recent consulting gig, I had to figure out what a particular program was doing. Usually this is not a huge issue, but this time it was a real pain. Finally I figured it out that instance variables were seemingly changed at random. Methods were constantly setting instance variables instead of using return values.
Remember that your goal is to make reading and writing code easy. Encapsulation is an important principle and, as in this case, not just about wrapping methods in a class. It's also about keeping the functionality for a method encapsulated within that single method if possible. Make it return a value don't just set an instance variable and then have the invoker use it like this:
String name = ""; compute(); System.out.println(name);
Instead, do this
String name = computer(); System.out.println(name);
I know this simple example seems obvious, but there are many programs that seem to rely on this modal or stateful programming. The less you have to keep in your mind at once (i.e., the less state information or context), the better.