Onions and Spaghetti — Lessons Learnt the Hard Way

I did a talk with this title recently. It doesn’t cover all of the points below, it was just a 30 min talk. The slides are here: attachment:onions&spaghetti.pdf. My friend Nick Seow pointed out this page to me: http://www.holub.com/goodies/rules.html, I agree with most of the points there too.

Designing your Code

Categorising Code

Three main types of code:

Model: Your data structures and algorithms, code that does computation and represents the internal state of your system.
View: A representation of your model. The output of the system as perceived by a user.
Controller: Code that ties the Model to the View. The pipes and infrastructure of your code.

Try to categorise each part of your code into one of the above. You may not have all 3 categories in your project, but most projects will. Try not to mix code from different categories. See MVC explained in a Cocoa framework for a good description of the design pattern.

Handling Exceptions

Almost universally handled badly
Exceptions: only catch them if you can do something about it.
Exceptions: if you can’t do anything about it, let it bubble up and fail. Fail-Fast code is good!.
Exceptions: don’t hot-potato exceptions. Try to deal with each exception only once.
Exceptions: can really complicate your program state if you keep on catching them. They change the program flow in unexpected ways.
Exceptions: If you put a big try statement around a lot of code, you are liable to not know which line raised the exception, and wont know how much of the state to roll back upon catching it. If you intend to roll back state, make your try statements as small as possible.

Global Variables Are Considered Evil.

We all learn this in Comp1A, but seem to forget it too frequently
Global variables masquerading as member variables are also Evil!

Why are global variables Evil?

Because they make tracking the program state HARD. Anything can mutate the state of global variables, making reasoning about them hard. You never know what they contain.

Because they make using multiple threads MORE dangerous and difficult

Because they make testing really really hard. Since there’s so much state to take care of, you end up having to write a lot of tests to cover it.

Controlling Data Flow

Make the flow of data through various areas of your system as simple as possible.
Try to avoid multi-directional data flow
Think Model-View-Controller when designing your program

Unspaghettifying your Objects.

Don’t use an object as a namespace, and its members as global variables.
Don’t be afraid of having a library of static functions that do computation. Not everything has to be an object.
Keep your object graphs well trimmed:

- Separate your logic from your object construction
- Avoid constructing new objects in your object - Dependency Injection will simplify your objects and make unit testing with mock objects easier.

Keep your inheritance graphs well trimmed
Keep your call graphs well trimmed. Don’t use long chains of function calls, they’re hard to follow and test.

Excessive Encapsulation is Evil

In school, we learn that encapsulation is good. A common trait in code is to encapsulate everything multiple times. Excessive encapsulation makes your code unreadable.
When writing internal interfaces, don’t be afraid to use public member variables directly. If you’re accessing a simple variable, such as the height of a rectangle or the IP of a device read from a config file, there’s usually no need to wrap it in a getter and setter. A getter and setter are essential when your variable isn’t simple; for example, when it has to be computed, or when there is access control.
Encapsulate logical parts of your code, eg: encapsulate a physical hardware device using an object that represents it. Don’t encapsulate basic computation or IO (eg: don’t encapsulate writing an int to a file).
Don’t encapsulate the encapsulated; e.g. if you have a config object already, usually there’s no point in wrapping it in another object which extracts data from the config class for the particular module. If you really need to do that, change your config object so that it makes separating out data for different modules easier.
Make your encapsulated objects general enough to use in different modules. If you have an object that represents a message, try to avoid having to subclass it for each module that sends messages. Make it general enough so all of them can send that type of message.
Remember every time you add another layer to your code, the reader of your code will have to look at another class, discover the relationship between the two, and verify her understanding of their relationship. Introducing extra code also means more testing is required, and increases the likelihood of introducing bugs.
Consider using composition rather than inheritance when possible to do so. Think about is-a and has-a relationships. Inheritance is often abused and can make code unreadable.
Avoid Onion Code . It will make your colleagues cry.

Threads are HARD, mmkay?

To offer another analogy, a folk definition of insanity is to do the same thing over and over again and expect the results to be different. By this definition, we in fact require that programmers of multithreaded systems be insane. Were they sane, they could not understand their programs. from The Problem With Threads

If it can be done with one thread, do it in one thread.
Keep the multi-threaded minority of your code separate from the single-threaded majority. Makes testing and debugging much much easier.
Processes and message passing are often better solutions. Shared state is hard to reason about (remember what I said about global variables?). If you want to share state, it’s better to explicitly pass it as messages, these are analogous to passing arguments rather than sharing global state.

Testing your code

Think about testing and maintenance before you start writing code.
Writing good tests is really difficult, but it does pay off. Write once, run many times! The more tests you write, the better you get at writing them quickly.
Try to write tests that are understandable by humans. This is quite difficult. Making your tests such that another programmer (or you in a couple of weeks) can follow them helps keep them up to date, and helps avoiding testing the wrong thing.
Don’t test the wrong thing! Easy to write a test that tests for an incorrect answer. If your test is human readable, it’s a good idea to have a colleague double check it.
Test the smallest unit of code possible (usually a single method), and try to test it only once. The higher up you go in your call graph, the more permutations are possible. If you try to test every combination from every input to every subsequent function call in each function, you will end up with a lot of tests that get broken as soon as someone makes the slightest change to your code. Testing at the lowest level means you don’t have to test as many permutations, making your test code size more manageable.
Badly written tests dramatically increase maintenance cost. Choose quality over quantity when testing.
Write test code for each bug you discover. Verify that it’s actually caused by what you think it is by reproducing it in the unit test, and then fix it and make sure the unit test passes. This means that you build up your test suite and guard against future changes breaking the functionality again, as well as quickly verifying your fix.
Test your test coverage with appropriate tools. lcov is available in the GCC compiler suite.
Integration testing is important. Make sure you have a staging environment where you can simulate real use scenarios.
Don’t just rely on unit tests, test your code with simulated data and lots of real data when possible. Have a framework for piping through data and verifying the output hasn’t changed (black box testing, regression testing).

In Summary

Minimise your state. The less state you need to consider, the easier it is to think about what’s happening. This is why pure functional programming is nice.
Make your code as simple as possible, but no simpler. Ask a colleague to review your code, see how she reacts to it. If she can understand it quickly, you get a tick. If she has trouble, you either need to make your code simpler, document it better, or both.
Control your data flow. Make interactions between modules/components as simple as possible.
Guard your code with good testing. Don’t rely on one sort of testing. Test your code with real data if possible.