CodeSOD: Grumpy Cat

At the end of the lecture session, students immediately started packing up their laptops to race across campus for their next class. Andrew's professor droned on, anyway, describing their assignment. "I've provided parser code," he said, "so you can just download the datafile and use it."

He had more to say, but no one was paying attention. Perhaps he even had a correction to the assignment- because when Andrew went to download the data file for the assignment 404ed.

Andrew emailed the professor, but figured- "Hey, I can get started. I'll just read the parser code to understand the file format."

String line = br.readLine();for (int i=0; i<result.length; i++) { line = br.readLine(); index = line.indexOf(" "); int number = Integer.parseInt(line.substring(0,index)); result[i] = number;}

The parsing code itself was an overly complicated reinvention of String.split with absolutely no error handling. That was enough to make Andrew cringe, but it's hardly the worst code you could have. No, the real bad code was the code that read from the file.

You might be thinking to yourself, "Well, Java does make file I/O a bit complicated, but you just link a FileInputStream with a BufferedReader, or even save yourself all that trouble and use the very convenient FileReader."

Now, I don't want to put words in this professor's mouth, but I think they would point out that a FileReader reinvents a wheel that's already on your OS. Why go through all this trouble to open a file and read from it if you already have a program on your OS that can open and read files?

List<String> command = new ArrayList<String>();command.add("cat");command.add(filename);ProcessBuilder builder = new ProcessBuilder(command);builder.redirectErrorStream(true);final Process process = builder.start();InputStream is = process.getInputStream();InputStreamReader isr = new InputStreamReader(is);BufferedReader br = new BufferedReader(isr);// discard the first lineString line = br.readLine();for (int i=0; i<result.length; i++) { line = br.readLine(); index = line.indexOf(" "); int number = Integer.parseInt(line.substring(0,index)); result[i] = number;}if (debugPrint) System.out.println("Program terminated?");int rc = process.waitFor();if (debugPrint) System.out.println("Program terminated!");

Why not open the cat command? I mean, sure, now you're tied to a system with cat installed, losing the cross-platform benefits of Java. I hope none of these students are using Windows. Then there's the overhead of launching a new process. Also, process.waitFor is a blocking call that doesn't terminate until the called program terminates, which means a bad input can hang the application.

It's worth noting that this code wasn't merely written for this assignment, either. In fact, this code started life as part of a research project conducted by multiple members of the CS department's faculty. Using this parser was not an optional part of the project, either- use of this code was required.

[Advertisement] Universal Package Manager - ProGet easily integrates with your favorite Continuous Integration and Build Tools, acting as the central hub to all your essential components. Learn more today!