Before our regularly scheduled programming, Code & Supply, a developer community group we've collaborated with in the past, is running a salary survey, to gauge the state of the industry. More responses are always helpful, so I encourage you to take a few minutes and pitch in.
Cid was recently combing through an inherited Java codebase, and it predates Java 8. That’s a fancy way of saying “there were no good date builtins, just a mess of cruddy APIs”. That’s not to say that there weren’t date builtins prior to Java 8- they were just bad.
Bad, but better than this. Cid sent along a lot of code, and instead of going through it all, let’s get to some of the “highlights”. Much of this is stuff we’ve seen variations on before, but have been combined in ways to really elevate the badness. There are dozens of these methods, which we are only going to look at a sample of.
Let’s start with the String getLocalDate()
method, which attempts to construct a timestamp in the form yyyyMMdd
. As you can already predict, it does a bunch of string munging to get there, with blocks like:
switch (calendar.get(Calendar.MONTH)){
case Calendar.JANUARY:
sb.append("01");
break;
case Calendar.FEBRUARY:
sb.append("02");
break;
…
}
Plus, we get the added bonus of one of those delightful “how do I pad an integer out to two digits?” blocks:
if (calendar.get(Calendar.DAY_OF_MONTH) < 10) {
sb.append("0" + calendar.get(Calendar.DAY_OF_MONTH));
}
else {
sb.append(calendar.get(Calendar.DAY_OF_MONTH));
}
Elsewhere, they expect a timestamp to be in the form yyyyMMddHHmmssZ
, so they wrote a handy void checkTimestamp
method. Wait, void
you say? Shouldn’t it be boolean?
Well here’s the full signature:
public static void checkTimestamp(String timestamp, String name)
throws IOException
Why return a boolean when you can throw an exception on bad input? Unless the bad input is a null, in which case:
if (timestamp == null) {
return;
}
Nulls are valid timestamps, which is useful to know. We next get a lovely block of checking each character to ensure that they’re digits, and a second check to ensure that the last is the letter Z
, which turns out to be double work, since the very next step is:
int year = Integer.parseInt(timestamp.substring(0,4));
int month = Integer.parseInt(timestamp.substring(4,6));
int day = Integer.parseInt(timestamp.substring(6,8));
int hour = Integer.parseInt(timestamp.substring(8,10));
int minute = Integer.parseInt(timestamp.substring(10,12));
int second = Integer.parseInt(timestamp.substring(12,14));
Followed by a validation check for day and month:
if (day < 1) {
throw new IOException(msg);
}
if ((month < 1) || (month > 12)) {
throw new IOException(msg);
}
if (month == 2) {
if ((year %4 == 0 && year%100 != 0) || year%400 == 0) {
if (day > 29) {
throw new IOException(msg);
}
}
else {
if (day > 28) {
throw new IOException(msg);
}
}
}
if (month == 1 || month == 3 || month == 5 || month == 7
|| month == 8 || month == 10 || month == 12) {
if (day > 31) {
throw new IOException(msg);
}
}
if (month == 4 || month == 6 || month == 9 || month == 11) {
if (day > 30) {
throw new IOException(msg);
}
"
The upshot is they at least got the logic right.
What’s fun about this is that the original developer never once considered “maybe I need an intermediate data structure beside a string to manipulate dates”. Nope, we’re just gonna munge that string all day. And that is our entire plan for all date operations, which brings us to the real exciting part, where this transcends from “just regular old bad date code” into full on WTF territory.
Would you like to see how they handle adding units of time? Like days?
public static String additionOfDays(String timestamp, int intervall) {
int year = Integer.parseInt(timestamp.substring(0,4));
int month = Integer.parseInt(timestamp.substring(4,6));
int day = Integer.parseInt(timestamp.substring(6,8));
int len = timestamp.length();
String timestamp_rest = timestamp.substring(8, len);
int lastDayOfMonth = 31;
int current_intervall = intervall;
while (current_intervall > 0) {
lastDayOfMonth = getDaysOfMonth(year, month);
if (day + current_intervall > lastDayOfMonth) {
current_intervall = current_intervall - (lastDayOfMonth - day);
if (month < 12) {
month++;
}
else {
year++;
month = 1;
}
day = 0;
}
else {
day = day + current_intervall;
current_intervall = 0;
}
}
String new_year = "" + year + "";
String new_month = null;
if (month < 10) {
new_month = "0" + month + "";
}
else {
new_month = "" + month + "";
}
String new_day = null;
if (day < 10) {
new_day = "0" + day + "";
}
else {
new_day = "" + day + "";
}
return new String(new_year + new_month + new_day + timestamp_rest);
}
The only thing I can say is that here they realized that “hey, wait, maybe I can modularize” and figured out how to stuff their “how many days are in a month” logic into getDaysOfMonth
, which you can see invoked above.
Beyond that, they manually handle carrying, and never once pause to think, “hey, maybe there’s a better way”.
And speaking of repeating code, guess what- there’s also a public static String additionOfSeconds(String timestamp, int intervall)
method, too.
There are dozens of similar methods, Cid has only provided us a sample. Cid adds:
This particular developer didn’t trust in too fine modularization and code reusing (DRY!). So for every of this dozen of methods, he has implemented these date parsing/formatting algorithms again and again! And no, not just copy/paste; every time it is a real wheel-reinvention. The code blocks and the position of single code lines look different for every method.
Once Cid got too frustrated by this code, they went and reimplemented it in modern Java date APIs, shrinking the codebase by hundreds of lines.
The full blob of code Cid sent in follows, for your “enjoyment”:
public static String getLocalDate() {
TimeZone tz = TimeZone.getDefault();
GregorianCalendar calendar = new GregorianCalendar(tz);
calendar.setTime(new Date());
StringBuffer sb = new StringBuffer();
sb.append(calendar.get(Calendar.YEAR));
switch (calendar.get(Calendar.MONTH)){
case Calendar.JANUARY:
sb.append("01");
break;
case Calendar.FEBRUARY:
sb.append("02");
break;
case Calendar.MARCH:
sb.append("03");
break;
case Calendar.APRIL:
sb.append("04");
break;
case Calendar.MAY:
sb.append("05");
break;
case Calendar.JUNE:
sb.append("06");
break;
case Calendar.JULY:
sb.append("07");
break;
case Calendar.AUGUST:
sb.append("08");
break;
case Calendar.SEPTEMBER:
sb.append("09");
break;
case Calendar.OCTOBER:
sb.append("10");
break;
case Calendar.NOVEMBER:
sb.append("11");
break;
case Calendar.DECEMBER:
sb.append("12");
break;
}
if (calendar.get(Calendar.DAY_OF_MONTH) < 10) {
sb.append("0" + calendar.get(Calendar.DAY_OF_MONTH));
}
else {
sb.append(calendar.get(Calendar.DAY_OF_MONTH));
}
return sb.toString();
}
public static void checkTimestamp(String timestamp, String name)
throws IOException {
if (timestamp == null) {
return;
}
String msg = new String(
"Wrong date or time. (" + name + "=\"" + timestamp + "\")");
int len = timestamp.length();
if (len != 15) {
throw new IOException(msg);
}
for (int i = 0; i < (len - 1); i++) {
if (! Character.isDigit(timestamp.charAt(i))) {
throw new IOException(msg);
}
}
if (timestamp.charAt(len - 1) != 'Z') {
throw new IOException(msg);
}
int year = Integer.parseInt(timestamp.substring(0,4));
int month = Integer.parseInt(timestamp.substring(4,6));
int day = Integer.parseInt(timestamp.substring(6,8));
int hour = Integer.parseInt(timestamp.substring(8,10));
int minute = Integer.parseInt(timestamp.substring(10,12));
int second = Integer.parseInt(timestamp.substring(12,14));
if (day < 1) {
throw new IOException(msg);
}
if ((month < 1) || (month > 12)) {
throw new IOException(msg);
}
if (month == 2) {
if ((year %4 == 0 && year%100 != 0) || year%400 == 0) {
if (day > 29) {
throw new IOException(msg);
}
}
else {
if (day > 28) {
throw new IOException(msg);
}
}
}
if (month == 1 || month == 3 || month == 5 || month == 7
|| month == 8 || month == 10 || month == 12) {
if (day > 31) {
throw new IOException(msg);
}
}
if (month == 4 || month == 6 || month == 9 || month == 11) {
if (day > 30) {
throw new IOException(msg);
}
}
if ((hour < 0) || (hour > 24)) {
throw new IOException(msg);
}
if ((minute < 0) || (minute > 59)) {
throw new IOException(msg);
}
if ((second < 0) || (second > 59)) {
throw new IOException(msg);
}
}
public static String additionOfDays(String timestamp, int intervall) {
int year = Integer.parseInt(timestamp.substring(0,4));
int month = Integer.parseInt(timestamp.substring(4,6));
int day = Integer.parseInt(timestamp.substring(6,8));
int len = timestamp.length();
String timestamp_rest = timestamp.substring(8, len);
int lastDayOfMonth = 31;
int current_intervall = intervall;
while (current_intervall > 0) {
lastDayOfMonth = getDaysOfMonth(year, month);
if (day + current_intervall > lastDayOfMonth) {
current_intervall = current_intervall - (lastDayOfMonth - day);
if (month < 12) {
month++;
}
else {
year++;
month = 1;
}
day = 0;
}
else {
day = day + current_intervall;
current_intervall = 0;
}
}
String new_year = "" + year + "";
String new_month = null;
if (month < 10) {
new_month = "0" + month + "";
}
else {
new_month = "" + month + "";
}
String new_day = null;
if (day < 10) {
new_day = "0" + day + "";
}
else {
new_day = "" + day + "";
}
return new String(new_year + new_month + new_day + timestamp_rest);
}
public static String additionOfSeconds(String timestamp, int intervall) {
int hour = Integer.parseInt(timestamp.substring(8,10));
int minute = Integer.parseInt(timestamp.substring(10,12));
int second = Integer.parseInt(timestamp.substring(12,14));
int new_second = (second + intervall) % 60;
int minute_intervall = (second + intervall) / 60;
int new_minute = (minute + minute_intervall) % 60;
int hour_intervall = (minute + minute_intervall) / 60;
int new_hour = (hour + hour_intervall) % 24;
int day_intervall = (hour + hour_intervall) / 24;
StringBuffer new_time = new StringBuffer();
if (new_hour < 10) {
new_time.append("0" + new_hour + "");
}
else {
new_time.append("" + new_hour + "");
}
if (new_minute < 10) {
new_time.append("0" + new_minute + "");
}
else {
new_time.append("" + new_minute + "");
}
if (new_second < 10) {
new_time.append("0" + new_second + "");
}
else {
new_time.append("" + new_second + "");
}
if (day_intervall > 0) {
return additionOfDays(timestamp.substring(0,8) + new_time.toString() + "Z", day_intervall);
}
else {
return (timestamp.substring(0,8) + new_time.toString() + "Z");
}
}
public static int getDaysOfMonth(int year, int month) {
int lastDayOfMonth = 31;
switch (month) {
case 1: case 3: case 5: case 7: case 8: case 10: case 12:
lastDayOfMonth = 31;
break;
case 2:
if ((year % 4 == 0 && year % 100 != 0) || year %400 == 0) {
lastDayOfMonth = 29;
}
else {
lastDayOfMonth = 28;
}
break;
case 4: case 6: case 9: case 11:
lastDayOfMonth = 30;
break;
}
return lastDayOfMonth;
}