String connection misuse
Wrong Writing:
String s = "";
for (Person p : persons) {
s += ", " + p.getName();
}
s = s.substring(2); //remove first comma
Correct writing:
StringBuilder sb = new StringBuilder(persons.size() * 16); // well estimated buffer
for (Person p : persons) {
if (sb.length() > 0) sb.append(", ");
sb.append(p.getName);
}
Wrong use of StringBuffer
Wrong Writing:
StringBuffer sb = new StringBuffer();
sb.append("Name: ");
sb.append(name + '\n');
sb.append("!");
...
String s = sb.toString();
The third line of the problem is that append char performs better than String, and the initialization StringBuffer does not specify a size, which may cause the internal array to resize when the intermediate append is used.If it is JDK1.5, it is best to replace StringBuffer with StringBuilder unless thread security is required.Another way is to connect strings directly.The disadvantage is to specify a length when it cannot be initialized.
Correct writing:
StringBuilder sb = new StringBuilder(100);
sb.append("Name: ");
sb.append(name);
sb.append("\n!");
String s = sb.toString();
Or write this:
String s = "Name: " + name + "\n!";
Test string equality
Wrong Writing:
if (name.compareTo("John") == 0) ...
if (name == "John") ...
if (name.equals("John")) ...
if ("".equals(name)) ...
The above code is good, but not good enough.compareTo is not concise enough, ==is meant to compare two objects to see if they are the same.In addition, it is better to judge the length of a character by comparing whether it is empty or not.
Correct writing:
if ("John".equals(name)) ...
if (name.length() == 0) ...
if (name.isEmpty()) ...
Convert Number to String
Wrong Writing:
"" + set.size() new Integer(set.size()).toString()
Correct writing:
String.valueOf(set.size())
Utilize immutable objects
Wrong Writing:
zero = new Integer(0);
return Boolean.valueOf("true");
Correct writing:
zero = Integer.valueOf(0); return Boolean.TRUE;
Use an XML parser
Wrong Writing:
int start = xml.indexOf("<name>") + "<name>".length();
int end = xml.indexOf("</name>");
String name = xml.substring(start, end);
Correct writing:
SAXBuilder builder = new SAXBuilder(false);
Document doc = doc = builder.build(new StringReader(xml));
String name = doc.getRootElement().getChild("name").getText();
Use JDom to assemble XML
Wrong Writing:
String name = ...
String attribute = ...
String xml = "<root>"
+"<name att=""+ attribute +"">"+ name +"</name>"
+"</root>";
Correct writing:
Element root = new Element("root");
root.setAttribute("att", attribute);
root.setText(name);
Document doc = new Documet();
doc.setRootElement(root);
XmlOutputter out = new XmlOutputter(Format.getPrettyFormat());
String xml = out.outputString(root);
XML Encoding Traps
Wrong Writing:
String xml = FileUtils.readTextFile("my.xml");
Because the encoding of the XML is specified in the file, the encoding must be specified when reading the file.Another problem is that you can't save an XML file with a String at a time, which can cause unnecessary waste of memory. The correct way to do this is to use InputStream while reading.To solve coding problems, it is best to use an XML parser.
No character encoding specified
Wrong Writing:
Reader r = new FileReader(file); Writer w = new FileWriter(file); Reader r = new InputStreamReader(inputStream); Writer w = new OutputStreamWriter(outputStream); String s = new String(byteArray); // byteArray is a byte[] byte[] a = string.getBytes();
Such code is mostly not cross-platform portable.Because different platforms may use different default character encoding.
Correct writing:
Reader r = new InputStreamReader(new FileInputStream(file), "ISO-8859-1");
Writer w = new OutputStreamWriter(new FileOutputStream(file), "ISO-8859-1");
Reader r = new InputStreamReader(inputStream, "UTF-8");
Writer w = new OutputStreamWriter(outputStream, "UTF-8");
String s = new String(byteArray, "ASCII");
byte[] a = string.getBytes("ASCII");
Data stream is not cached
Wrong Writing:
InputStream in = new FileInputStream(file);
int b;
while ((b = in.read()) != -1) {
...
}
The above code is a byte-by-te read, which results in frequent local JNI file system access and is very inefficient because calling local methods is time consuming.It's best to wrap it in BufferedInputStream.A test was done to read 1MB from/dev/zero, which took about 1s, and packaged in BufferedInputStream only required 60ms, resulting in 94% performance improvement! This also applies to output stream operations and socket operations.
Correct writing:
InputStream in = new BufferedInputStream(new FileInputStream(file));
Unlimited heap memory use
Wrong Writing:
byte[] pdf = toPdf(file);
There is a premise here that the file size cannot talk about JVM heap burst.Otherwise, wait for OOM, especially for server-side code that is highly concurrent.The best practice is to use Stream to read and store (local files or databases).
Correct writing:
File pdf = toPdf(file);
Additionally, for server-side code, at least the file size needs to be limited for system security.
Do not specify timeout
Error code:
Socket socket = ... socket.connect(remote); InputStream in = socket.getInputStream(); int i = in.read();
This has happened more than once at work.Personal experience generally does not exceed 20s timeout.There is a problem here. connect can specify a timeout, but read cannot.However, you can set the blocking time.
Correct writing:
Socket socket = ... socket.connect(remote, 20000); // fail after 20s InputStream in = socket.getInputStream(); socket.setSoTimeout(15000); int i = in.read();
In addition, file reads (FileInputStream, FileChannel, FileDescriptor, File) cannot specify timeouts, and IO operations involve local method calls, which further manipulate the JVM's control. In a distributed file system, IO operations are actually network calls inside.In general, operations that operate for 60s can be considered to have timed out.To solve these problems, caching and asynchronous/message queuing are commonly used.
Frequent use of timers
Error code:
for (...) {
long t = System.currentTimeMillis();
long t = System.nanoTime();
Date d = new Date();
Calendar c = new GregorianCalendar();
}
Each new Date or Calendar involves a local call to get the current time (although this local call is faster than other local method calls).
If time is not particularly sensitive, the clone method is used here to create a new Date instance.This is more efficient than direct new.
Correct writing:
Date d = new Date();
for (E entity : entities) {
entity.doSomething();
entity.setUpdated((Date) d.clone());
}
If looping takes a long time (more than a few ms), you can create a Timer immediately and update the timestamp periodically based on the current time, 200 times faster on my system than a direct new time object:
private volatile long time;
Timer timer = new Timer(true);
try {
time = System.currentTimeMillis();
timer.scheduleAtFixedRate(new TimerTask() {
public void run() {
time = System.currentTimeMillis();
}
}, 0L, 10L); // granularity 10ms
for (E entity : entities) {
entity.doSomething();
entity.setUpdated(new Date(time));
}
} finally {
timer.cancel();
}
Catch all exceptions
Wrong Writing:
Query q = ...
Person p;
try {
p = (Person) q.getSingleResult();
} catch(Exception e) {
p = null;
}
This is a query operation for EJB3 and the possible reasons for the exception are: the result is not unique; there is no result; the database is inaccessible and all exceptions are caught and set to null to mask all exceptions.
Correct writing:
Query q = ...
Person p;
try {
p = (Person) q.getSingleResult();
} catch(NoResultException e) {
p = null;
}
Ignore all exceptions
Wrong Writing:
try {
doStuff();
} catch(Exception e) {
log.fatal("Could not do stuff");
}
doMoreStuff();
There are two problems with this code, one is that the caller was not told that the system call was wrong. The second is that the log has no cause for the error and it is difficult to track the location problem.
Correct writing:
try {
doStuff();
} catch(Exception e) {
throw new MyRuntimeException("Could not do stuff because: "+ e.getMessage, e);
}