Bush league is a general term used to describe an action or thing as being amateur, inferior or crude. In a literal sense, it refers to a low quality minor-league in baseball not associated with any of the major league teams. The term originated from the state of minor-league fields that often were ringed with shrubs and bushes.
- Wikipedia
int value = getValue();
String valStr = value + "";
// Eh, I was too lazy to type Integer.toString(value)
In addition to obfuscating the intent of the code, this results in unnecessary object creation. While modern JVMs lessen the expense of unnecessary object creation, it's not zero.
// TODO: make this a property or something
File dataFile = new File("/home/joedeveloper/projects/testdata.dat");
Hard-coding file paths is okay one-off toy applications, but not production-ready systems. At a minimum, you shouldn't require your fellow developers to edit source code just to get an application up and running. And you absolutely should not require a new binary release to support a file system change.
At least, make the path configurable via a command-line Java system property (i.e. -Dmyapp.datafile=/home/joedeveloper/testdata.dat).
ProtocolMessge msg = null;
if (type == ORDER) {
msg = buildOrderMessage();
} else if (type == CANCEL) {
msg = buildCancelMessage();
}
// might be null...but that's somebody else's problem!
return msg;
If you don't have a better solution than pre-initializing a variable to null, you probably don't understand the code. Also, you should strive not to return null from methods (aka "passing the buck") - unless your application is littered with defensive null checks, you're just asking for a NullPointerException.
Exception for Exceptions
Sometimes you need to initialize the variable so you can handle it in a finally block, i.e. closing output streams that have thrown an Exception. See Sun Java Tutorial example.
String zeroPaddedNum = num + "";
for (int i = 0; i < leadingZeros; i++) {
zeroPaddedNum = "0" + num;
}
// I hope the young generation GC isn't already full!
// You get that GC memory and CPU cycles for free anyway...AMIRITE?
Same as above - the cost of unnecessary object creation may have been minimized in recent releases of Java, but it's still not zero. Also, java.text.NumberFormat solves this particular problem a lot more succinctly.
See:
java.net: "Oh, go ahead. Prematurely optimize"
Ok, this is kind of nitpicking, but if a class isn't used outside its package, it and all its member methods should be "package-private" (aka default access).
Making everything public is like inviting somebody else to mis-use your class a year from now. The only classes that should be able to see it are the classes that use it.
public HelperClass {
public void doSomethingImportant() {
// warning: this kills performance for a few seconds
}
public void reset() {
// reload persistence...
}
public void reloadConfig() {
// etc...
}
}
public class AppStats {
// why oh why can't java have a "global" keyword
private static int counter;
public AppStats {
// re-read counter from persist
counter = recoverCounterFromPersist();
}
public void count() {
counter++;
}
}
This is actually a version of Singleton Simpleton pattern. Makes code difficult to follow and prone to concurrency issues (count() needs to be synchronized).
public class MySingleton {
private static String value;
/** Make sure to call this first before using this singleton! */
public static void init() {
value = loadValueFromFile();
}
public static String getValue() {
return value;
}
private static loadValueFromFile() {
// imagine file IO code here...
}
}
Again, difficult to follow and error prone. What if init() gets called multiple times from multiple threads? Eliminating Singletons from your code and using dependency injection can prevent these errors from even being a possibility.
It's 2008 2013 and people are still indicating exceptional conditions in Java with "return codes" instead of...throwing an exception.
Why?!?
private double calcualteOrderValue(String symbol, int quantity) {
Double price = symbolToPriceMap.get(symbol);
if(price == null) {
return -1;
} else {
return price * quantity;
}
}
If you don't feel comfortable throwing an exception from a method, at least have the method return an ADT instead of a primitive (i.e. CalculationResult).
private void fetchOrder(ProtocolMessage msg) {
// need to retrieve something from a map...
String key = msg.getConnectionName() + "~" + msg.getDirection() + "~" + msg.getSessionId() + "~" + msg.getSequenceNumber();
return orderMap.get(key);
}
This is bug prone due to the possibility of conflicting keys (what if the connection name includes a Bigger problem is the unnecessary string concatenation operation when you could just contrive a key class.~ character)?
If you don't want to create your own light-weight key class, use Apache Commons Collections: MultiKey
