Is Your Code Worthy Of Being Improved

ExplicitYou know that feeling you get when you open up a piece of code you’ve never seen before and it’s a complete and total morass of crap. It’s that sinking feeling in your stomach when, no matter how big your screen is, the code just doesn’t seem to fit. You don’t get even a glimmer of indication of what the code is meant to do by giving it a quick skim and the unit test are, if anything, worse than the code (crappy unit tests are worse than crappy code). You know you will need to start at the beginning and go through this sucker line by line to pry even a semblance of meaning out of it. I really hate that feeling and I hate the people who make me feel like that (of course I don’t really hate them, but you sure do wish you could go all Homer Simpson on them – when he chokes Bart – just for a couple of minutes). What I would hate most of all though, is for someone else to feel like that about me and my code.

We’ve all heard that old maxim about leaving code in a slightly better state than when you found it, but you know what, sometimes you feel like the code is so disgusting that you just want to get out of there as soon as you can. It’s a bad situation for a piece of code to end up in, not conducive to being improved, not worthy. And I am well aware that sometimes, depending on what you’re working on, it is damn tough to write pretty code. But, here is the way I view it, at the very least, your code should be worthy of improvement! I want people to feel at home when they look at my code, but failing that I’d like them to not immediately want to throw up when they come for a visit. In my mind it all starts with being explicit.

Being Explicit When You Code

We all have our little quirks when we code and for the most part this is fine, but there are certain conventions that tend to clarify the meaning and we should all endeavor to adopt them if we can.

  • descriptive variable names – it is not that difficult to use full words, we are not writing assembly code. So everything will be a little longer, but – fPtr vs. filePointer– which one is easier to understand? Maybe both of those are pretty clear, but what if you have a fullyAutomatedWidgetFactory vs. flAutWidFact, all of a sudden the sailing is not as smooth.
  • descriptive method names –  ditto for this one, make the method name tell the story of what it does (do I even need to mention class names that make sense)
  • structuring, classes and method in a way that makes sense – private and public methods mixed together in classes, classes themselves broken up along arbitrary boundaries. It takes very little time to extract chunks of code that have a relationship and make sense (when you refactor) and you will understand your code better yourself.
  • classes of reasonable size – this one is just a no-brainer, if you need to scroll for 5 minutes to find anything, it’s crying out to be split up and simplified.
  • methods of reasonable size – another no brainer, if it can’t fit on one 19 inch screen, it is long overdue for a refactor (and I am being extremely generous).
  • no long trains of method calls – chaining 20 method calls together – not a good idea. It might make perfect sense to you because you know the code, but to someone else it is just groan-worthy. Be sensible, have some compassion for your fellow developers.
  • etc.

There are dozens more I can list, but seriously, it if looks bit suss it probably is! Sometimes following some of these will actually prevent you from using all the funky features/shortcuts of your favorite language, but I believe it is a small price to pay for a little bit of extra clarity and readability. And I am aware that most of those are basics and yet so many people just don’t do it, which is just not nice. Not nice for those who have to maintain and modify the code later.

Using Technologies That Obfuscate Meaning

Sometimes in the interests of being cool and cutting edge or simply because it seems like a good idea, we use technologies that obfuscate our code beyond the level that anyone should reasonably be forced to endure. Here are my two pet examples of this phenomenon.

  • Spring Autowiring – I can’t really think of a situation where this is a good idea. Sure it might make wiring up your integration tests a little easier, but only in the short term. Wait until half the people on your team are new and see how much they like it.
  • Aspect Oriented Programming (AOP) – don’t get me wrong, the idea of AOP is just great. And to be honest there are a some specialized situations where it is a good idea to use it judiciously (transactions, security etc.). But, use it without thinking and you make your code an order of magnitude more complicated than it has to be. It was designed for a purpose don’t try to retrofit it into somewhere it’s not meant to be.

Every time you’ve got a new library you want to try out or want to make your code a little bit more ‘hip’, stop and think. Is it adding to the clarity of the solution or detracting from it? If you had to come in and work with this code would this make your life easier or harder? If the answer is harder, find another way to do what you want. This is software, there is always another way, and if it happens to be a little bit more verbose, well maybe that’s fine. If the verboseness is adding readability, then bring it on.

The Other Side Of Convention Over Configuration

Convention over configuration is great, I have nothing against it. When we all use the same conventions it makes everyone’s life easier. But what happens when the convention doesn’t suit your needs any more and you have to break it? The more you do this the dirtier and ‘hackier’ your code will seem to those who are used to the convention. So, if you’re working within a system where convention over configuration is the norm, think long and hard before you have to break the convention. Perhaps if you find yourself doing this often early on, it may not be too late to make a different technology choice, where the conventions are not as strict. Of course, sometimes you will not have the luxury, in that case do try to stick to the convention, diverge only when you have no other choice. Always remember to think of those who will come after you when you make decisions, after all, you probably want them to think of you when they make theirs.

These are just my thoughts, to me, being explicit is important. You might have different ideas on what constitutes ‘nice’ code, but if you get nothing else out if it I hope you will at least agree with this. The way to get others to leave your code in a better state than they found it, is to make it worthy of improvement in the first place! If we all strive towards that, ultimately, everyone will be better off for it.

Image by wanderingYew2

  • I would recommend reading Clean Code by Uncle Bob.

  • Pingback: Dew Drop – August 12, 2009 | Alvin Ashcraft's Morning Dew()

  • Travis Dunn

    > The way to get others to leave your code in a better state than they found it, is to make it worthy of improvement in the first place!

    i.e. lead by example, and allow yourself to be shamed (I mean “inspired”) into improvement whenever you encounter elegantly structured code.

    • Excellent advice, and whenever you do encounter nice code, always remember to take something away from it, don’t just admire and go back to doing things the way you always have.

  • Pingback: Daily Links for Thursday, August 13th, 2009()

  • Greg

    Other than your ill thoughtout rant against spring and aspectj I completely agree with you about code “style”. When ever you have an object that needs to access an application scope resource then it is bad for that object to know too much about how to get that object, or even to have some knowledge of how it is created. Most people use singletons or factory classes for these cases. Using spring, aspectj, and Autowiring are perfect for these situations. The Object just knows it needs an X and when it gets created it has an X. The creation and configuration of X is done completely independently from any of the objects that depend on it, and this makes your code much easier to read, write, and debug. And if you use Interfaces properly and have a well designed API then you can swap out implementations of X seamlessly. Thats just one situation, that from what I have seen happens quite often.

    • I don’t have anything against dependency injection in general, it is only the autowiring aspect that I have a problem with. When you’re trying to figure out what a piece of code is doing, if everything is autowired, you have to carefully trace what what is actually occurring, if everything is explicit your configuration becomes a great help rather than a useless file.

      • Mark

        Try using Transaction Demarcation without Autowiring. I hardly use autowiring, but I do in this instance. It removes tons of config info. I currently only use it for Services (injecting the repositories) and Repositories (injecting the persistance manager). It is pretty obvious what is going on because I am using Annotations.

        As for AOP – yes use it wisely. Same for IoC. These are powerful concepts. ORM is about the same. The problem is that in not using them the result is code bloat and thus reduces maintainability.

        Other than that, I totally agree. I think you’d be fine with the Autowiring the things i mentioned.

        Funny how I spent the last few days making up a list of things we need to cover in walkthroughs (no, we don’t have them NOR pair programming) for my new team. It matches a bunch of what you said.

        As for new people – they don’t like anything. And if they always remain new people … time for new people. LOL.

        • I’ll have a look at autowiring for demarcating transactions, always happy to give my ‘prejudices’ a once over :).

          And you’re right judicious use is the order of the day, don’t walk blindly into using any non-trivial library, know why you need it and how to apply it.

          Funny you should mention new people, I started drafting up some of my thoughts regarding that last night :). You’re right though, new people will tend to always question everything, but you expect that to settle down to manageable levels eventually, if it doesn’t something is wrong either with the people or with the team/environment (it is a distinct possibility).