CodeSOD: Self-Documented

Molly's company has a home-grown database framework. It's not just doing big piles of string concatenation, and has a bunch of internal checks to make sure things happen safely, but it still involves a lot of hardcoded SQL strings.

Recently, Molly was reviewing a pull request, and found a Java block which looked like this:

if (Strings.isNullOrEmpty(getFilter_status())) {sql.append(" and review in ('g','t','b','n','w','c','v','e','x','')");} else if (!"a".equals(getFilter_status())) {sql.append(" and review = '"+getFilter_status()+"'");} else {limit_results=true;}

"Hey, I get that the database schema is a little rough, but that big block of options in that in clause is incomprehensible. Instead of magic characters, maybe an enum, or at the very least, could you give us a comment?"

So, the developer responsible went back and helpfully added a comment:

if (Strings.isNullOrEmpty(getFilter_status())) {sql.append(" and review in ('g','t','b','n','w','c','v','e','x','')"); // f="Resolved", s="Resolved - 1st Call"} else if (!"a".equals(getFilter_status())) {sql.append(" and review = '"+getFilter_status()+"'");} else {limit_results=true;}

This, of course, helpfully clarifies the meaning of the f flag, and the s flag, two flags which don't appear in the in clause.

Before Molly could reply back, someone else on the team approved and merged the pull request.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.