My Code Is Bad And I Should Feel Bad : What I Learned From A Code Review

So, I’m fairly young. I’m 21 years old. There are a lot of people out there who are significantly smarter than I, and have a hell of a lot more life experience than I.

Nothing is more true when it come to programming. There are some people out there who are just mindblowingly amazing coders. You see them, and you just want to channel Wayne Campbell and go all:

And I think that the best way to improve as a programmer (or as anything really) is if you speak to these people and ask them to look at your code and to give you advice and direction.

So, I asked my close friends Roland Hesz, Dewi Morgan and Frans De Jonge to look at some Java code I produced and asked them to give me a code review and to be honest and brutal and to give me constructive feedback. I wasn’t disappointed.

This article will look at some things I picked up from my code review, and how I plan to improve.

My Project – Java!Java!JSON!

It’d likely be worthwhile to explain what I’m trying to actually accomplish first. So, I’m a student of Computing at Liverpool Hope University. In our first year, we learn C and Java, and we we have to make a product in each of these languages. In C, we make a video game or a physics simulation. In Java, we can make whatever we want.

I decided to make a toy compiler and VM. The project is called Java!Java!JSON!, and is named so because it interprets a specially structured JSON document which is then passed into a toy VM written in Java, which runs on the Java VM.

It’s nothing awe inspiring. It’s a fairly basic project that I’m working on because I enjoy finding unique ways to abuse JSON and because I thought it’d be fun.

Getting Feedback

I sent off my code (handily stored on my Github profile) and waited. Each of my friends then promptly responded with some amazing, clear, actionable feedback. Here’s what I learned.

Parlez-vous Java?

So, I’m no Java developer, and I don’t have the deepest knowledge of the Java API. Most of my experience is in other languages, like PHP and Ruby. As a result, I had code that was a bit like this:

System.out.println(“File located at: ” + Directory + “/” + OutputName + “.JJJ”);
// Obviously the above is only true on a *Nix system. I should eventually work out how to determine the user’s OS.

Can you see the problem there? If I ran my code on a Windows system, it wouldn’t run because Windows uses backslashes in its path declarations.

Fortunately, Java has a nice little built in method called File.separator which actually determines what OS is running and what file separator type is appropriate. This does pretty much all the leg-work required. I didn’t  know about this until Frans told me about it.

From this, I realized that I should sit down and get a better, more in-depth understanding of how Java works and learn the useful little facets of the Java programming language.

Source Code Only A Mother Could Love

There were parts of my code that were actually not really hideous. Functions were terse and clearly named. I made sure they did one thing, and one thing only. They obeyed Curly’s Law. Capitalization was really consistent. On the whole, I was generally quite consistent.

But, there were bits of the source code that could be more aesthetically pleasing. Firstly, I could have changed my capitalization to reflect scope and to reflect the contemporary coding styles in Javaesque languages.

Whilst my functions were quite clear in what they did, I didn’t really comment all that much. Yeah, I know it’s really damn important. In particular, there was no commenting in my Main method. This really, really needs to be fixed as you can’t really gleam what the program does just by looking at the Main Method.

Cancelling The Housekeeping

So, in my code, I had a few methods that would call System.exit(1) whenever they’d hit a critical problem. This works like the little red ‘Emergency Stop’ buttons on trains, and just ends the program without doing anything else.

Whilst this is fine for a tiny little toy program like mine, it’s generally bad practice. What if your program opens a DB connection? Are you just going to leave a bunch of connections open? This is a major design and implementation issue.

In my case, what if my program fails whilst interpreting the JSON file and has already started to write to the pseudo-bytecode file? It’ll just quit without tidying things up, or without giving any meaningful information as to why things went wrong.

The solution? I’m going to write a class that handles what to do when things go all Pete Tong. Some logging will be involved. I’ll try to close things more gracefully. I’ll try to tell the user where things went wrong

Fin

Zed Shaw in his infamous ‘Rails Is A Ghetto’ rant said something that really resonated with me:

“Great programmers don’t defend stupid, they stamp it out and own up to their mistakes.”

I’m not a great programmer. Nowhere near. I’d like to be one. The only way to be a great programmer is to identify where you’re screwing up and where you’re going right, and to make concerted efforts to get better. And I’m really grateful that I have such brilliant friends who are prepared to talk to me in depth about the code I produce. To Frans, Roland and Dewi: Thanks ever so much!

Help Me Out

My project isn’t even slightly finished, but I welcome feedback and advice. The source to the VM is here and the source to the compiler is here. I’d be honored if you had a look, and told me what you thought.

Like this article?

You should subscribe. Pop your email in the box in the sidebar and you’ll get an email whenever I write a new post.

About Matthew Hughes

Matthew Hughes is a software developer, student and freelance writer from Liverpool, England. He is seldom found without a cup of strong black coffee in his hand and absolutely adores his Macbook Pro and his camera. You should follow him at @matthewhughes.

3 comments

  1. I love the title. Code reviews should be liberating and help you see things you missed or could simplify. It looks like you received that kind of feedback.

    Of course code reviews as part of a job, when you are sitting in conference room with your co-workers and boss tend to be slightly more stressful.

Leave a Reply