Layered Like Spaghetti – The Daily WTF

“We use a three tier architecture,” said the tech lead on Cristian‘s new team. “It helps us keep concerns separated.”

This statement, as it turned out, was half true. They did divide the application into three tiers- a “database layer”, a “business layer”, and a “presentation layer”. The “database layer” was a bunch of Java classes. The “business layer” was a collection of Servlets. And the “presentation layer” was a pile of JSP files.

What they didn’t do, however, was keep the concerns separated.

Here’s some code from their database layer:

public synchronized StringBuffer getStocTotGest(String den, String gest) {
		StringBuffer sb = new StringBuffer("<table width=\"100%\"  border=\"1\" cellspacing=\"1\" cellpadding=\"1\">" + "<tr bgcolor=\"#999999\">" + "<td>Denumire</td>" + "<td>Cant</td>"
				+ "<td>PretVanz</td>" + "</tr>");
		try {
			ResultSet rs = connectionManager
					.executeQuery("select (if(length(SUBSTRING(den,1,instr(den,'(')-1))>0,SUBSTRING(den,1,instr(den,'(')-1),den)) as den,um,pret_vinz,sum(stoc) as stoc from stmarfzi_poli where den like '"
							+ den + "%' " + gest + "  group by den  order by den");
			while (rs.next()) {
				sb.append("<tr><td>" + rs.getString("den") + "</td>");
				sb.append("<td><div align=\"right\">" + threeDecimalPlacesFormat.format(rs.getDouble("stoc")) + " " + rs.getString("um") + "</div></td>");
				sb.append("<td><div align=\"right\">" + teoDecimalPlacesFormat.format(rs.getDouble("pret_vinz")) + "</div></td></tr>");
			}
			sb.append("</table>");
		} catch (Exception ex) {
			ex.printStackTrace();
		}
		return sb;
	}

I guess a sufficiently motivated programmer can write PHP in any language.

This just has a little bit of everything in it, doesn’t it? There’s the string-munged HTML generation in the database layer. The HTML is also wrong, as header fields are output with td tags, instead of th. There’s the SQL injection vulnerability. There’s the more-or-less useless exception handler. It’s synchronized even though it’s not doing anything thread unsafe. It’s truly a thing of beauty, at least if you don’t know what beauty is and think it means something horrible.

This function was used in a few places. It was called from a few servlets in the “business layer”, where the resulting StringBuffer was dumped into a session variable so that JSP files could access it. At least, that was for the JSP files which didn’t invoke the function themselves- JSP files which mixed all the various layers together.

Cristian’s first task in the code base was changing the background colors of all of the rendered table headers. Since, as you can see, they weren’t using CSS to make this easy, that involved searching through the entire codebase, in every layer, to find all the places where maybe a table was generated.

Changing those colors was Cristian’s first task in the code base. I assume that Cristian is still working on that, and will be working on that for some time to come.