The Daily WTF: Curious Perversions in Information Technology

2010-08-23 Reply Admin

The Article:
Had the developer taken the time to clean up this function, they might have noticed that they never closed any of the JDBC connection resources defined in the method.

No, probably not. Normally you just put that in a finally block at the very beginning and that takes care of it. This guy probably didn't know any better.

Airhead · 2010-08-23 Reply Admin

Closing connections, opening connections... Who has the time for such nonsense? Just keep them open!

If connection limit is reached, a) raise the limit b) add new hardware and c) do some magic.

2010-08-23 Reply Admin

Fixed?

public static boolean isWidgetReferenced(int widgetId) {
	// should be static, obviously (unless this is in a Singleton class)

	try {
		// leaving this in here so memory isn't allocated if this is an infrequently-called method
		String[] tables = new String[] {
 "WIDGET_REGION", "WIDGET_OFFERING", "WIDGET_ORDERS",
 "WIDGET_NEW", "WIDGET_HISTORY", "WIDGET_STATS_RECORDED", 
 "WIDGET_VIEWED", "WIDGET_SETS", "WIDGET_INSTALLED", 
 "WIDGET_PENDING", "WIDGET_JOURNEL" };

		Connection connection = getConnection();
		ResultSet rs;
		for (String table : tables) {
			String query = "SELECT COUNT(*) FROM " + table + " WHERE widgetId=" + widgetId;
			rs = connection.createStatement().executeQuery(query);
			int count = rs.getInt(1);
			if (count > 0) return true;			
		}		
	} catch (Exception e) {
		// stupid, but in theory could be fixed with alterations to log4j.properties
		logger.error("isWidgetAvailable", e);
	} finally {
		connection.close();
	}

	return false;
}

Lipstick on a pig

Someone You Know · 2010-08-23 Reply Admin

Scott Selikoff:
...he managed to shave 15 minutes off his commute...

If fifteen minutes is just a "shave" off your commute, your commute is way too fucking long.

2010-08-23 Reply Admin

Not bragging, but I'm working on an application whose lead developer insists on writing code just like that... from "don't close connection" down to "we use datareaders for scalar stuff" and "stored procedures are for sissies".

Captcha: immitto. Those guys are imitating us fo sho.

Cbuttius · 2010-08-23 Reply Admin

The Nerve:

Fixed?

public static boolean isWidgetReferenced(int widgetId) {
	// should be static, obviously (unless this is in a Singleton class)

	try {
		// leaving this in here so memory isn't allocated if this is an infrequently-called method
		String[] tables = new String[] {
 "WIDGET_REGION", "WIDGET_OFFERING", "WIDGET_ORDERS",
 "WIDGET_NEW", "WIDGET_HISTORY", "WIDGET_STATS_RECORDED", 
 "WIDGET_VIEWED", "WIDGET_SETS", "WIDGET_INSTALLED", 
 "WIDGET_PENDING", "WIDGET_JOURNEL" };

		Connection connection = getConnection();
		ResultSet rs;
		for (String table : tables) {
			String query = "SELECT COUNT(*) FROM " + table + " WHERE widgetId=" + widgetId;
			rs = connection.createStatement().executeQuery(query);
			int count = rs.getInt(1);
			if (count > 0) return true;			
		}		
	} catch (Exception e) {
		// stupid, but in theory could be fixed with alterations to log4j.properties
		logger.error("isWidgetAvailable", e);
	} finally {
		connection.close();
	}

	return false;
}

Lipstick on a pig

Better but not what I'd called fixed, still having thought about it let's keep it slow and sluggish, making them open a connection every time and run 11 SQL queries. We have to give them some incentive to upgrade, I suppose.

2010-08-23 Reply Admin

The Nerve:

Fixed?

public static boolean isWidgetReferenced(int widgetId) {
	// should be static, obviously (unless this is in a Singleton class)

	try {
		// leaving this in here so memory isn't allocated if this is an infrequently-called method
		String[] tables = new String[] {
 "WIDGET_REGION", "WIDGET_OFFERING", "WIDGET_ORDERS",
 "WIDGET_NEW", "WIDGET_HISTORY", "WIDGET_STATS_RECORDED", 
 "WIDGET_VIEWED", "WIDGET_SETS", "WIDGET_INSTALLED", 
 "WIDGET_PENDING", "WIDGET_JOURNEL" };

		Connection connection = getConnection();
		ResultSet rs;
		for (String table : tables) {
			String query = "SELECT COUNT(*) FROM " + table + " WHERE widgetId=" + widgetId;
			rs = connection.createStatement().executeQuery(query);
			int count = rs.getInt(1);
			if (count > 0) return true;			
		}		
	} catch (Exception e) {
		// stupid, but in theory could be fixed with alterations to log4j.properties
		logger.error("isWidgetAvailable", e);
	} finally {
		connection.close();
	}

	return false;
}

Lipstick on a pig

...except you screwed the logic and return true if there's at least one row in any of the tables, instead of at least one row in all of them...

2010-08-23 Reply Admin

Surely TRWTF is that a function named "isWidgetReferenced" is actually returning 'available'? - that is, whether the passed widget number is NOT referenced?

2010-08-23 Reply Admin

Who knows, maybe getConnection() enables some kind of connection pooling? That said, the fetching is kind of weird.

toth · 2010-08-23 Reply Admin

the article:
if (0 < count) return false;

So if count is greater than zero, the widget is not referenced? What, do the tables keep track of places where the widget doesn't occur?

2010-08-23 Reply Admin

No, if it's greater than 0, it means that the widget is not available.

2010-08-23 Reply Admin

kjordan:
Who knows, maybe getConnection() enables some kind of connection pooling? That said, the fetching is kind of weird.

good point

2010-08-23 Reply Admin

If all you want to do is checkj for the existence of at least on occurrence of a value in a table then you'd usually do well to do something like (in Oracle):

select
  count(*)
from
  my_table
where
  my_column = :my_bind and
  rownum = 1

Could save you do all the work to count the other ten million rows that you don't care about.

Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.

2010-08-23 Reply Admin

Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.

Markp · 2010-08-23 Reply Admin

Anonymous:
Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.

We've been waiting for Java 7 for quite a while now. It'll have something similar to "using" for automatic resource management.

2010-08-23 Reply Admin

Anonymous:
Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.

That sounds like a neat feature, but I don't think there's plans to incorporate anything like that in the future.

That said, it's not necessary these days to manually close connections for most applications: they're moving toward dependency injection (IoC pattern) for the pooled resources.

tiller · 2010-08-23 Reply Admin

Slim Dave:
If all you want to do is checkj for the existence of at least on occurrence of a value in a table then you'd usually do well to do something like (in Oracle):
select
  count(*)
from
  my_table
where
  my_column = :my_bind and
  rownum = 1
Could save you do all the work to count the other ten million rows that you don't care about.

Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.

Doing it that way may have really bad performance on some databases if you use transactions, because then count(*) is not a simple operation. (You can't just return the number of rows because an other transaction may have a lock on some of them and thus delete them (This prevent the database from just returning the number of rows)).

So I think that doing a select with limit 1 and then testing for an empty result is a better solution.

Cbuttius · 2010-08-23 Reply Admin

Anonymous:
Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.

It supports finalizers for objects, not quite the same as destructors that you have in C++.

The actual method in which connections are handled here should be transparent, but when you let go of your object it should either close the connection or return it to the pool for reuse, dependent on which method was used to retrieve it.

The final object's finalizer can be implemented to do exactly that and you won't have to "remember" to do it yourself. Of course, the problem is you don't know exactly when this will happen, i.e. it is not guaranteed to happen as you leave scope, unlike in C++ where it is.

2010-08-23 Reply Admin

tiller:
Slim Dave:
If all you want to do is checkj for the existence of at least on occurrence of a value in a table then you'd usually do well to do something like (in Oracle):
select
  count(*)
from
  my_table
where
  my_column = :my_bind and
  rownum = 1
Could save you do all the work to count the other ten million rows that you don't care about.

Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.
Doing it that way may have really bad performance on some databases if you use transactions, because then count(*) is not a simple operation. (You can't just return the number of rows because an other transaction may have a lock on some of them and thus delete them (This prevent the database from just returning the number of rows)).

So I think that doing a select with limit 1 and then testing for an empty result is a better solution.

Yep, quite possibly you're right -- this is an Oracle-specific solution, and Oracle would stop retrieving rows once the first has been found. And readers don't block writers in Oracle either of course. Syntactical and locking differences -- another reason for using stored procedures?

2010-08-23 Reply Admin

Anonymous:
Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.

TRWTF is that people who don't know to type

finally {
  resource.close();
}

will probably not be educated enough to type

  using (Connection conn = connection ...

The code is only as smart as the smartest idiot.

2010-08-23 Reply Admin

Larry Lard:
Surely TRWTF is that a function named "isWidgetReferenced" is actually returning 'available'? - that is, whether the passed widget number is NOT referenced?

I was thinking the same thing... there's nothing like trying to puzzle out the real meaning of an "opposite named function".

'commoveo' - commoveo and fix this function name you idiot!

2010-08-23 Reply Admin

TRWTF (although a terrifyingly common one) is catching the base Exception class and treating all failures as the same.

In this case, if the database is down, the code will act as if it has found a widget in the database, which could have all kinds of interesting unintended consequences in the logic down the line.

2010-08-23 Reply Admin

The Nerve:

Fixed?

	try { ...
		Connection connection = getConnection();
              ...
	} finally {
		connection.close();
	}

No.

2010-08-23 Reply Admin

Markp:
Anonymous:
Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.
We've been waiting for Java 7 for quite a while now. It'll have something similar to "using" for automatic resource management.

Yep, that's basically it but they've opted to extend the try..catch construct instead of implementing a new keyword. It's all just syntactic sugar and I must admit I agree with the Java philosophy that syntactic sugar is bad (the proposal notes: "like all syntactic sugar, this construct removes a bit of Java’s 'what you see is what you get' character"). But it's a small change that offers big returns in terms of code quality - if it's actually used...!

Severity One · 2010-08-23 Reply Admin

From the article:

Paul wasn't sure what bothered him the most: that the developer had used a strangely formed for() loop on a structure that could return at most one record [...]

Well, it's a funky way to do a while loop; from what I remember, this is actually how the JVM implements a while loop.

As in regards to the other comments, about Java not having any destructors and such: true, it could do with those, but apart from that, JDBC is a bit of a mess when it comes to design. You always end up with an enormous amount of boilerplate code just to do a simple query.

2010-08-23 Reply Admin

Severity One:
From the article:
Paul wasn't sure what bothered him the most: that the developer had used a strangely formed for() loop on a structure that could return at most one record [...]
Well, it's a funky way to do a while loop; from what I remember, this is actually how the JVM implements a while loop.
As in regards to the other comments, about Java not having any destructors and such: true, it could do with those, but apart from that, JDBC is a bit of a mess when it comes to design. You always end up with an enormous amount of boilerplate code just to do a simple query.

TRWTF is that Java does not have a while loop, which would do the same thing without all the extra punctuation.

2010-08-23 Reply Admin

Larry:
TRWTF is that Java does not have a while loop, which would do the same thing without all the extra punctuation.

It's been a long time since I've done Java but even I can tell you that it definitely has a while loop!

2010-08-23 Reply Admin

Severity One:
From the article:
Paul wasn't sure what bothered him the most: that the developer had used a strangely formed for() loop on a structure that could return at most one record [...]
Well, it's a funky way to do a while loop; from what I remember, this is actually how the JVM implements a while loop.

It is not a while loop written using the for statement. It is an if statement written using the for statement. See the unconditional break at the end of the body?

2010-08-23 Reply Admin

Someone You Know:
Scott Selikoff:
...he managed to shave 15 minutes off his commute...

If fifteen minutes is just a "shave" off your commute, your commute is way too fucking long.

Or you commute is that unpredictable.

My commute? 20 min. if you hit every green light along the way, 90 min. if everyone else decided to commute at the exact same time as you.

2010-08-23 Reply Admin

The Web is the Root of All Info:
Someone You Know:
Scott Selikoff:
...he managed to shave 15 minutes off his commute...

If fifteen minutes is just a "shave" off your commute, your commute is way too fucking long.

Or you commute is that unpredictable.

My commute? 20 min. if you hit every green light along the way, 90 min. if everyone else decided to commute at the exact same time as you.

Hmm. My commute is about two chapters of a typical science fiction novel. I love public transit!

Mason Wheeler · 2010-08-23 Reply Admin

Slim Dave:
Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.

Standard Yoda Conditional for you. The people who invented C had more marketing skill than technical competence, and they made a compiler that thinks anything can be a boolean and called it a "feature." And now all the descendant languages are stuck with it and all the ugly syntax that it spawned, such as writing conditionals backwards.

2010-08-23 Reply Admin

wtf:
The Web is the Root of All Info:
Someone You Know:
Scott Selikoff:
...he managed to shave 15 minutes off his commute...

If fifteen minutes is just a "shave" off your commute, your commute is way too fucking long.

Or you commute is that unpredictable.

My commute? 20 min. if you hit every green light along the way, 90 min. if everyone else decided to commute at the exact same time as you.

Hmm. My commute is about two chapters of a typical science fiction novel. I love public transit!

My commute is normally the time that I like to shave with my portable razor, brush my teeth (I use a water bottle for my rinse), and apply deodorant. You've probably met me if you ride the bus enough.

2010-08-23 Reply Admin

TRWTF is that nobody has recommended a database view. It will certainly simplify the process of adding the missing table references by not requiring a new build/deploy.

2010-08-23 Reply Admin

The Web is the Root of All Info:
Or you commute is that unpredictable.
My commute? 20 min. if you hit every green light along the way, 90 min. if everyone else decided to commute at the exact same time as you.

How would my luck with green lights / competing traffic affect your commute time?

Under perfect conditions, my commute is 15 minutes; if the lights conspire against me and I hit rush hour, I'm looking at 90 minutes. Thankfully I'm paid for getting the job done, not for occupying a desk 40 hours a week.

2010-08-23 Reply Admin

Henning Makholm:

The Nerve:

Fixed?

	try { ...
		Connection connection = getConnection();
              ...
	} finally {
		connection.close();
	}

No.

He's also not closing the result set or the statement.

Cbuttius · 2010-08-23 Reply Admin

Actually the issue was that they used = to mean assign and == to mean compare so coders may accidentally put

if( x = 0 )

but it will compile, assign x to 0 and evaluate to false and fail at runtime if the coder meant

if( x == 0 )

Therefore to avoid this error someone came up with the idea of forcing people to write

if( 0 == x )

so that if you forget and accidentally write

if( 0 = x )

it will fail at compile time as you can't assign 0.

It may have been a better idea in hindsight to use := for assignment and = for comparison which is perhaps more intuitive although coders would still have got it wrong, probably using = when they meant to assign, and most likely would still get code that compiles but fails to work.

luis.espinal · 2010-08-23 Reply Admin

Anonymous:
Does Java still not support the "using" pattern for disposable objects? I'm no fan of pointless syntactic sugar but this pattern would be a useful addition to the language. I like Java a lot but in all honesty I'm grateful to be working on .NET these days.

I'm a fan of strategically used syntactic sugar, but the "using" keyword/clause could possibly do more harm than good in this particular context. The reason is that an idiot/incompetent programmer will still be an idiot/incompetent one with or without it.

Let's look at some of the original codesod:

rs = connection.createStatement().executeQuery(query);

It is not only the creation of a java.sql.Connection object, but the creation of a great many number of java.sql.Statement (we only need one) and many java.sql.ResultSet objects, with none of them being cleared, re-used or closed.

Imagine the dumbass who wrote that code being equipped with a Java equivalent of "using". Assuming his pineapple-level IQ gets him to realize "shit, I need to close them thingies", wrapping everything in "using" clauses, we would have:

Even more redundant, unreadable, obfuscated code, and
Would not compensate for the dumbass' inability to understand basal things like object indefinite lifetimes in Java or the fundamental fact that resources have cost and are limited.

Adding "dispose pattern" syntactic sugar will not help here because the problem is deep, systemic and systematic. Syntactic sugar is like neosporing, and dumbass programmers are like skin cancer. You can't use the former to effectively treat the later.

2010-08-23 Reply Admin

Mason Wheeler:
Slim Dave:
Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.

Standard Yoda Conditional for you. The people who invented C had more marketing skill than technical competence, and they made a compiler that thinks anything can be a boolean and called it a "feature." And now all the descendant languages are stuck with it and all the ugly syntax that it spawned, such as writing conditionals backwards.

if((moron > idiot) || (idiot < moron)) { . . . }

luis.espinal · 2010-08-23 Reply Admin

Mason Wheeler:
Slim Dave:
Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.

Standard Yoda Conditional for you. The people who invented C had more marketing skill than technical competence, and they made a compiler that thinks anything can be a boolean and called it a "feature." And now all the descendant languages are stuck with it and all the ugly syntax that it spawned, such as writing conditionals backwards.

Well, to their credit, the ability to evaluate things to "0" and evaluate it as false comes very handy for pointer arithmetic and manipulation.

I highly doubt that the people who write conditionals backwards do it for a C influence. I'd put on a lack of analytical skills and subpar understanding of code aesthetics (CS stuff that no one should be allowed to graduate without.)

Put a folk like that under stress till they go way over their head, and voila, you get code like that in no time.

JamesQMurphy · 2010-08-23 Reply Admin

The Nerve:
The Article:
Had the developer taken the time to clean up this function, they might have noticed that they never closed any of the JDBC connection resources defined in the method.
No, probably not. Normally you just put that in a finally block at the very beginning and that takes care of it. This guy probably didn't know any better.

I put my finally blocks at the end. But that's just me.

Scott Selikoff · 2010-08-23 Reply Admin

The Nerve:

Fixed?

for (String table : tables) {
	String query = "SELECT COUNT(*) FROM " + table + " WHERE widgetId=" + widgetId;
	rs = connection.createStatement().executeQuery(query);
	int count = rs.getInt(1);
	if (count > 0) return true;			
}

Lipstick on a pig

Bug: You never called rs.next(), as required in JDBC to read a record from a result set.

Normally, rs.next() is called inside of a while() loop for multiple records, and inside an if() statement for a single record. In this case, the if() statement is not required to call rs.next(), since you expect exactly one row every time (even with 0 records, the query should return a single row with value 0), although it is good practice to always wrap rs.next() with a conditional clause to avoid reading an invalid result set.

2010-08-23 Reply Admin

The Nerve:

Fixed?

public static boolean isWidgetReferenced(int widgetId) {
	// should be static, obviously (unless this is in a Singleton class)

	try {
		// leaving this in here so memory isn't allocated if this is an infrequently-called method
		String[] tables = new String[] {
 "WIDGET_REGION", "WIDGET_OFFERING", "WIDGET_ORDERS",
 "WIDGET_NEW", "WIDGET_HISTORY", "WIDGET_STATS_RECORDED", 
 "WIDGET_VIEWED", "WIDGET_SETS", "WIDGET_INSTALLED", 
 "WIDGET_PENDING", "WIDGET_JOURNEL" };

		Connection connection = getConnection();
		ResultSet rs;
		for (String table : tables) {
			String query = "SELECT COUNT(*) FROM " + table + " WHERE widgetId=" + widgetId;
			rs = connection.createStatement().executeQuery(query);
			int count = rs.getInt(1);

		if (count > 0) return true;	</pre></span><pre>		
	}		
} catch (Exception e) {
	// stupid, but in theory could be fixed with alterations to log4j.properties
	logger.error("isWidgetAvailable", e);
} finally {
	connection.close();
}

return false;

}

Lipstick on a pig

You just broke the API and all of the unit tests will fail now. The code as posted returns true only if the Widget is NOT referenced. I think the method was written on opposite day. Alternative theory, the method was originally isWidgetUnrefereneced() but someone pointed out that it should be named to reflect positive logic in accordance with the project coding standards. The developer renamed it without inverting the result (so s/he would not break the unit tests).

sstair · 2010-08-23 Reply Admin

Funny, one of the first things I noticed was that the method isWidgetReferenced returns true if the widget ISN'T referenced (and is therefore available), and false if it IS reference.

hoodaticus · 2010-08-23 Reply Admin

Frits is right; the advisablility of moving the multiple table lookups into a union really depends. For instance, the code short-circuits lookups if count != 0 on any given lookup. The database will not short-circuit the union.

For code clarity, if the db side of it was fast, I would throw it all into a union. If I needed performance, I would use multiple, short-circuiting lookups as was done here.

But I would never use a for "loop" in that manner.

2010-08-23 Reply Admin

This code needs a FOR-CASE statement. ;)

public boolean isWidgetReferenced(int widgetId) {

	boolean available = false;
	String query;
	ResultSet rs = null;
	int count = 0;

	try {
		Connection connection = getConnection();
		
		for (int i = 0; i < 12; i++)
		{
			switch (i) {
			case 0: 
				query = "SELECT COUNT(*) FROM WIDGET_REGION WHERE widgetId=" + widgetId;
				break;
			case 1: 
				query = "SELECT COUNT(*) FROM WIDGET_OFFERING WHERE widgetId=" + widgetId;
				break;
			case 2: 
				query = "SELECT COUNT(*) FROM WIDGET_ORDERS WHERE widgetId=" + widgetId;
				break;
			case 3: 
				query = "SELECT COUNT(*) FROM WIDGET_NEW WHERE widgetId=" + widgetId;
				break;
			case 4: 
				query = "SELECT COUNT(*) FROM WIDGET_HISTORY WHERE widgetId=" + widgetId;
				break;
			case 5: 
				query = "SELECT COUNT(*) FROM WIDGET_STATS_RECORDED WHERE widgetId=" + widgetId;
				break;
			case 6: 
				query = "SELECT COUNT(*) FROM WIDGET_VIEWED WHERE widgetId=" + widgetId;
				break;
			case 7: 
				query = "SELECT COUNT(*) FROM WIDGET_SETS WHERE widgetId=" + widgetId;
				break;
			case 8: 
				query = "SELECT COUNT(*) FROM WIDGET_INSTALLED WHERE widgetId=" + widgetId;
				break;
			case 9: 
				query = "SELECT COUNT(*) FROM WIDGET_PENDING WHERE widgetId=" + widgetId;
				break;
			case 10: 
				query = "SELECT COUNT(*) FROM WIDGET_JOURNAL WHERE widgetId=" + widgetId;
				break;
			case 11: 
				query = "SELECT COUNT(*) FROM WIDGET_JOURNAL WHERE widgetId=" + widgetId;
				break;
			}
			
			rs = connection.createStatement().executeQuery(query);
			for (; rs.next();) {
				count = rs.getInt(1);
				break;
			}
			if (0 < count) return false;
		}
		available = true;
	} catch (Exception e) {
		logger.error("isWidgetAvailable", e);
	}

	return available;
}

Captcha: sino. Code looks much better, si? No.

2010-08-23 Reply Admin

Java (and I think dotNet) require the if expression to be a boolean (bool).

if (x = 5) will evaluate to an int and fail at compile time.

However

boolean answer = f(); if (answer = true)

would still suffer from the same problem, assign true to answer and then evaluate to true.

Of course, with booleans, I always think the comparison operator is unnecessary.

if (answer == true) can be changed to if (answer) if (answer == false) can be changed to if (!answer)

I am pretty sure that James G. would never write if (answer == true) over if (answer) and probably did not even consider it when designing Java.

2010-08-23 Reply Admin

Mad Adder:
This code needs a FOR-CASE statement. ;)

Your version has material code inside the loop that is executed in more than one iteration. Therefore it is not the true for-case pattern.

The utility of "switch instead of a constant array" is in general much more debatable than the canonical for-case construction.

2010-08-23 Reply Admin

Slim Dave:
Also, "(0 < count)" ... is that irritating to anyone else? "(count > 0)" seems a lot more intuitive to me.

Anyone we learns to read right to left first will not have a problem with this.

I worked with a Jordanian who had Arabic as his first language. He called "<" "greater than" because he learned it from right to left in his elementary math class. It made perfect sense as long as he read the whole clause right to left. The problem was he would read the clause left to right, but still read the symbol as learned right to left.

bidi bidi bom bom

2010-08-23 Reply Admin

If the database was properly normalized with cascading updates/deletes, then you should only have to check the primary widget table.

Failure to exist in the widget table should indicate failure to exist in the entire DB!

Of course, from the code seen here, this developer may have still messed it up.

2010-08-23 Reply Admin

anonymous:
TRWTF is that nobody has recommended a database view. It will certainly simplify the process of adding the missing table references by not requiring a new build/deploy.

Yes, this is much neater, much quicker, and still as completely wrong as the original (and all proposed variants I've seen).

This whole thing is a ticking time bomb, just add users to speed up the clock.

8:30 a.m. Sally checks if WIDGET_ID = 5 is available. It is. 8:31 a.m. Tom checks if WIDGET_ID = 5 is available. It is.
8:32 a.m. Sally adds WIDGET_ID = 5 to some WIDGET table.
8:33 a.m. Tom adds WIDGET_IT = 5 to some other WIDGET table.

Uh-oh.

This is one of those great bugs that remains hidden during all unit testing, is completely unseen during the initial rollout to a small user base, and blows up in your face on the fateful afternoon all 250 of your users are scrambling to add their widgets before month end (you know, the one time you really, really do not want your application to blow up).

Correct solution: when you need a new WIDGET_ID, get it from a sequence-generator (in Oracle, <sequence_name>.NEXTVAL).

Swallowed by the Beast

Leave a comment on “Swallowed by the Beast”