Forums
Code Snippet Of the Day (CodeSOD) features interesting and usually incorrect code snippets taken from actual production code in a commercial and/or open source software projects.
Peter was tracking down some bugs, when he found code which looks like this:
Jul 24if (IsEmptyOrNull(myInput))
{
// do things that clearly expect myInput to NOT be null or empty
} else {
throw BadInputException("The input must not be null.");
}
Lisa was working on a project she quite liked. The tech was cool, the problems being solved were interesting, and the team had a good working process. Company-wide, however, budgets were tight, and other projects were in much worse shape, so the project Lisa was on got put on pause, and her team was moved onto a different project.
Someone wanted to make sure that functions which had large side effects were only called in the right places. Now, most of us might use some mixture of public/private, clear documentation, and maybe some key flags and error checking to ensure this was the case.
Today's anonymous submitter, who we'll call Sally, works on medical devices. As you can imagine, the development process for such devices is very strict. I mean, it should be, but we know that the industry has problems.
Unfortunately for Sally, one of those problems is the tech lead on a project she is inheriting. Said tech lead is leaving, and Sally is coming on to replace them. The project is in C#, and Sally is the most experienced with the language, making her the obvious choice to step in.
Today's anonymous submitter found some Java code which finds the largest value in a quartet of floats. Now, the code is quite old, so it actually predates varargs in Java. That doesn't excuse any of what you're about to see.
public float CalculateMaximumValue(float a, float b, float c, float d) {
int i = 0;
float[] arr = new float[] { 0, 0, 0, 0 };
float gtval = 0;
for (i = 0; i < 4; i++) {
arr[i] = 0;
}
arr[0] = a;
arr[1] = b;
arr[2] = c;
arr[3] = d;
gtval = arr[0];
for (i = 0; i < 4; i++) {
if (arr[i] > gtval) {
gtval = arr[i];
}
}
return gtval;
}
Kendall found some recently written code, and had to wonder, "Who wrote this crap?" Much to Kendall's disappointment, source control knew who wrote it: Kendall.
if (x < 0.0)
{
x += 0.0 - x;
width -= 0.0 - x;
}
Sebastian started a new job recently. Like a lot of "I started a new job," stories, this one starts with a 1,000 line class definition. What's notable about this one, however, is that most of that code is error handling. Now, you might think to yourself, "well, there's nothing inherently wrong with loads of error handling, if the problem calls for it.
This code is getting posted here. Do you think the problem calls for it?
We all know that building SQL queries via string concatenation, and then sending them to the database, is just begging for fragile code and SQL injection attacks. But, what if the bad part is the "sending them to the database" part? Has anyone ever thought about that?
Kris's predecessor has.
Many years ago, Sam was obeying Remy's Law of Requirements Gathering ("No matter what your requirements actually say, what your users want is Excel") and was working on a web-based spreadsheet and form application.
The code is not good, and involves a great deal of reinvented wheels. It is, for example, Java based, but instead of using any of the standard Java web containers for hosting their code, they wrote their own. It's like Java Servlets, but also is utterly unlike them in important and surprising ways. It supports JSP for views, but also has just enough surprises that it breaks new developers.
One of the first things anyone learns about object oriented programming is the power of inheritance and overriding functions. Isn't it great that you can extend or modify the implementation of a function in your derived classes? Don't you wish you could just do that for every function? Aash's co-worker certainly does.
@Override
public boolean isEnabled() {
if (!super.isEnabled()) {
return false;
}
return true;
}
"Dark Horse" inherited some PHP code. They had a hundred lines to submit, but only sent in a dozen- which is fine, as the dozen lines tell us what the other hundred look like.
$suite_1_1 = number_format($item -> {'suite_1_1_'.$the_currency}, 2, '.', '');
$suite_1_2 = number_format($item -> {'suite_1_2_'.$the_currency}, 2, '.', '');
$suite_1_3 = number_format($item -> {'suite_1_3_'.$the_currency}, 2, '.', '');
$suite_1_4 = number_format($item -> {'suite_1_4_'.$the_currency}, 2, '.', '');
$suite_2_1 = number_format($item -> {'suite_2_1_'.$the_currency}, 2, '.', '');
$suite_2_2 = number_format($item -> {'suite_2_2_'.$the_currency}, 2, '.', '');
$suite_2_3 = number_format($item -> {'suite_2_3_'.$the_currency}, 2, '.', '');
$suite_2_4 = number_format($item -> {'suite_2_4_'.$the_currency}, 2, '.', '');
$suite_3_1 = number_format($item -> {'suite_3_1_'.$the_currency}, 2, '.', '');
$suite_3_2 = number_format($item -> {'suite_3_2_'.$the_currency}, 2, '.', '');
$suite_3_3 = number_format($item -> {'suite_3_3_'.$the_currency}, 2, '.', '');
$suite_3_4 = number_format($item -> {'suite_3_4_'.$the_currency}, 2, '.', '');
Sean was supporting a web application which, as many do, had required form fields for the user to fill out. The team wanted to ensure that the required fields were marked by an "*", as you do. Now, there are a lot of ways to potentially accomplish the goal, especially given that the forms are static and the fields are known well ahead of time.
The obvious answer is just including the asterisk directly in the HTML: <label for="myInput">My Input(*)</label>: <input…>. But what if the field requirements change! You'll need to update every field label, potentially. So someone hit upon the "brillant" idea of tracking the names of the fields and their validation requirements in the database. That way, they could output that information when they rendered the page.
Today's anonymous submitter asks a question: "How do you imagine the rest of the codebase to be like?"
Well, let's look at this snippet of TypeScript and think about it:
Today's anonymous submitter inherited an application with a huge list of bugs and feature requests for missing features. While tracking down a bug, our submitter learned a lot about why "Allow additional stores to be viewable in the store selector," was an unimplemented feature.
if (inv.inv_storeloc == 0) {
out.println("<option selected value=\"0\">Select</option>");
out.println("<option value=\"1\">Location 1</option>");
out.println("<option value=\"2\">Location 2</option>");
} else if (inv.inv_storeloc == 1) {
out.println("<option selected value=\"1\">Location 1</option>");
out.println("<option value=\"2\">Location 2</option>");
} else {
out.println("<option value=\"1\">Location 1</option>");
out.println("<option selected value=\"2\">Location 2</option>");
}