I often catch myself doing code unnecessarily complicated (sad but true). Is there any set of rules, discipline, etc to help avoid that? (excluding the KISS principle, because I know it but often forget when it comes to the real code :)

@ThorbjørnRavnAndersen - +50. I have a boss that keeps saying that about my code and he can't seem to come up with any examples or reasons why it's complicated. Most concrete he seemed to get is complaining about using aggregate initializers instead of memset!
–
Crazy EddieMay 20 '12 at 15:38

Just stop adding new features to it and move to next class/module/library.
–
tp1May 20 '12 at 16:15

@tp1 That is probably not the best solution. Moving stuff to other modules won't solve anything. It may actually make things worse, as you lose the connection between actually connected things. Only separate features if they have no (close) connection.
–
FUZxxlMay 20 '12 at 20:04

6 Answers
6

As Antoine de St-Exupéry says: "A designer knows that he has achieved perfection not when there is nothing left to add, but when there is nothing left to take away."

This is a difficult and common problem, but it's something you can address over the course of the next few years. Recognizing it's a problem is the most important step, and once you have learnt to correct the kinds of complications you tend to add, you'll start designing without them.

When I realized I had this problem early in my career, I found it helpful to analyze and reanalyze every class, function, member variable, local variable, and nested function calls to see if things could be consolidated or eliminated while still meeting the given requirements. Basically, code golf.

Before you start to code, make a plan and design an interface for your code. Always think about what your code is actually going to do and what it probably isn't. Be realistic about that, but don't use any quirky solutions. Each function in the interface should do exactly one thing, if possible what it does should only depend on its arguments.

A good measure to see whether your code is overcomplicated is deepness of nesting. Once my code is nested five or so levels deep, I know that something went wrong. Try to split your code into functions that express several steps of what you want to do. Expect the compiler to generate efficient code for you. Compilers are smart.

Split your code into modules that represent functional units of your program. Each module should contain a set of functions that are likely to be used together. Neither throw unrelated things into one module nor write a group of modules that are always used together.

Don't implement functionality you won't need, but keep in mind that one might want to expand your program

Maybe you should have a look at some literature about extreme programming. Those guys are very much trying to avoid unnecessary complexities. A common source for complexities is trying to generalize even if there is only one application of the generalized whatever. I think Kent Beck recommended to only generalize something if you have already implemented it in three different ways. For the first tries you should "do the simplest thing that could possibly work".

I'd like to think that I am pretty good at keeping things simple, at least when using C#, even though stack exchange is full of people who are better than me. You get better with experience. I personally do not like to plan ahead too much, particularly for small tools because I do not really know which exact direction I will move in until I sink my teeth into the project. So, I have to have something semi-working first. Once it does and I see no more potential hard spots, then I start trying to simplify things. Now, the easiest way to simplify things is to keep the UI and features simple to begin with. I have been making prototypes recently so sometimes it is easy for me to decide what goes in and what does not. If you have to give estimates and the code goes right into the core and will be highly scrutinized by QA, then you probably cannot afford the time to make everything perfect because of deadlines plus the quality better be good. In this situations you WANT to keep things simple in order to avoid/minimize the stress of missed deadlines.

If you have more freedom, and you want to delight the users while not taking a year to do it, then ask people for specific feedback. Do a random hall usability testing to make sure that other people can use the tool. Consult with more experienced colleagues on what a good approach might be. Implement something that works and post pieces on the code review site to see how others can solve the same problem better.

If you are about to decide between library A, library B or rolling your own - ask on Stack Overflow for advice. If you still are learning something such as CSS or SQL or XAML - ask on Stack Overflow.

You want to be breaking a problem up into pieces, find a good solution for each sub-problem and then put it all together.

Along the way you should utilize tools for re-factoring, style checking, TDD, etc.

For C# I would recommend Resharper, StyleCop. Try to find something for your language. Basically, you want to use your time efficiently so that you will have time for "post-mortem", refactoring, creating a library that

Once you are in the simplification/perfection stage, you are never quite done. This sucks if you have to move onto another project and not ever touch this code again unless fixing a bug. If the stakes are not as high, then you can go back to the older project and remove lines and lines of code once you discover generics, some library, etc. Did I mention that Stack Overflow is a great resource? Just do not be a http://slash7.com/2006/12/22/vampires/.

You can learn a lot from re-factoring old code, so that you will avoid making the same mistakes in the future. I hope this helps.

Even when I think I'm doing a good job of writing a program, chances are it can be simplified a lot.

I find the most useful thing to do is ask "Regardless of how it does it, what is this program supposed to accomplish?"
i.e. If it's a compiler, it is supposed to read a file of code and write something that can execute. As such, if it's doing nothing unnecessary, it will be I/O bound.

I've also found that performance tuning leads to simplifying, as in this example, so if you get pretty good at diagnosing performance and recoding to improve it, that alone will teach you what you may be doing that is needlessly complicated, so you learn not to do it.
(Often it is what you've been taught to do.)

That's as simple as it gets. If we follow the KISS principle then we might do something like this. The function is NOT simple to test since now we have to redirect standard output to something the tests can process. You fix this by adding complexity to the function's interface by taking the output handle as a parameter. This is not as simple as can be done and I've run into KISS adherents that really freak out over stuff like this...especially if they're fighting with their superiors about whether unit testing should be done.

Furthermore, what is "simple" is in the eye of the beholder. Just the other day I caught shit for using a buffer object that grows as needed rather than malloc+memset+memcpy to copy a string handed to me by a network API. In my opinion using a buffer object is simpler because it's already there, does what I need (albeit it will do more), etc... In the other guy's opinion doing it by hand is simpler because the buffer is overkill. He's of course right because he's the boss.

Yet more, when faced with the decision to use a global variable to allow access by many functions vs. passing this variable around as a parameter it is quite obvious that the former is much "simpler". It's also rather obvious that in most cases it is the bad choice. Adherents to KISS though pick the former and their code turns into this untested knot of spaghetti that isn't even possible to conceptualize since modularity and decoupling have all gone out the window in an effort to keep things "simple"...and yes, by an obvious and possibly naive measure of simple it is much simpler to reach out and grab touch global than it is to call a function existing only to hide this global or pass around a parameter.

Very simple, that. I've seen way too many KISS adherents writing C code that looks like that, including the one I've currently got hammering on everything I do as "complicated". They assume so damn much and get way simpler than is safe to. Anyone who knows the language knows that the function above is incredibly unsafe and WILL someday result in bugs that take hours and hours to resolve. It's code like the above that give C and C++ bad names in fact and the industry is riddled with shit like this from basic apps to the financial industry and others...it's terrifying to me. But this is "simple" and why would you write a bunch of error detection and buffer protection code when you know that the input is always going to be shorter than X?

The answer to that question is of course that someone isn't going to know that you have this internal and arbitrary size limit and will exceed it...or some bug up the line will--which is what happened.

After catching crap for the last 8 months from this KISS adherent I've become rather cynical of the idea and think it probably leads to more problems than catches them. Writing reusable code is a good thing. Writing tested code is a good thing. Decoupling is a VERY good thing...and I would assert that in the end they make things easier to manage, easier to read, etc.. They are not, however "simple"; at least not in the way I've seen that word used of late.