The Austin Test: A software checklist for silicon vendors

^photo by John Robert Shepherd under CC-BY-2.0

Silicon chips are usually made by chipmakers^{[citation needed]}. These are companies largely staffed by chip designers — who often, quite naturally, view the chip as the product. But a more holistic view would say that the product is the value that the chip provides to their customer — and for all but the simplest passive parts, that value is provided partly through software.

Software design is a related but decidedly different skill-set to chip design. That means that the difference between silicon products succeeding or failing, can be nothing to do with the merit or otherwise of the chip design itself, but instead down to the merit or otherwise of the accompanying software. A sufficiently large chip buyer, aiming their own product at a sufficiently lucrative market, can often overcome poor “developer experience” by throwing internal software engineering at the problem: for a product that eventually sells in the millions, it can indeed be worth shaving cents off the bill-of-materials cost by specifying a chip that’s cheaper but harder-to-use than the alternative, even when taking into account the increased spending on software development.

But if you’re a chipmaker, many of your customers will not be in that position — or may accept that they are, but still underestimate the software costs, and release the product in a bug-ridden state or not at all — and ultimately sell fewer of their own products to end-users and thus buy fewer of your chips.

So the product isn’t finished just because the chip itself is sitting there, taped-out and fabbed and packaged and black and square and on reels of 1,000. For any complex or high-value chip — a microcontroller, for instance — the product is not complete until there’s also a software story, usually in the form of a Software Development Kit (SDK) and accompanying documentation. But a chipmaker staffed unremittingly and at too high a level with only chip-design experts may not even, corporately, realise when the state of the art in software has moved on. So here is a checklist — in the spirit of the famous “Joel Test” — to aid hardware specialists in assessing the maturity of a chipmaker’s software process; I’ve named it after the home-town of a chipmaker I once worked for.

The Austin Test Could your SDK have been written using only your public documentation? Is your register documentation generated from the chip source? Are the C headers for your registers, generated from the chip source? Are your register definitions available in machine-readable form (e.g. SVD)? Is your pinmux definition available in machine-readable form? Are all the output pins tristated when the chip is held in reset, and on cold boot? Is it straightforward to get notifications when your public documentation changes (e.g. new datasheet revision released)? Is it straightforward to use your SDK as a software component in a larger system?

1. Could your SDK have been written using only your public documentation?

Sometimes when the silicon comes back from the fab, it doesn’t work quite the way the designers expected. There’s no shame in that. And quite often when that happens, there’s a software workaround for whatever the issue is, so you put that workaround into your SDK. There’s no shame in that either. But if the problem and the workaround are not documented, you’ve just laid a trap for anyone who’s not using your entire SDK in just the way you expected. (See Questions 4 and 8.)

Perhaps your customer is using Rust, or Micropython. Perhaps they have a range of products, based on a range of different chips, of which yours is just one. If there’s “magic” hidden in your C SDK to quietly work around chip issues, then those customers are going to have a bad time.

(The original Mastodon post of which this blog post is a lengthy elaboration.)

2. Is your register documentation generated from the chip source?

I’m pretty sure that even the chipmakers with the most mature and sensible software operations don’t actually do this: they don’t have an automated process that ingests Verilog or VHDL and emits Markdown or RTF or some other output that gets pasted straight into the “Register Layout” sections of their public documentation. (You can sort of tell they don’t, from the changelogs in reference manuals.) But it’s the best way of guaranteeing accuracy — certainly superior to having human beings painstakingly compare the two.

Because I’ve worked at chip companies, I do realise that one reason not to do this is because not everything is public. Because silicon design cycles are so long, and significant redesign is so arduous, what chipmakers do is speculatively design all manner of stuff onto the chip, then test it and only document the parts that actually work or that they can find uses for. Sometimes this is visible as mysterious gaps in memory maps or in peripheral-enable registers; sometimes it’s less visible. I can personally vouch that there is a whole bunch of stuff in the silicon of the Displaylink DL-3000 chip that has never actually been used by the published firmware or software. But this is easily dealt with by equipping the automated process with a filter that just lets through the publicly-attested blocks. It’s still a win to have an automated process for the documentation just of those blocks!

3. Are the C headers for your registers, generated from the chip source?

This is again essentially the question, do you have a process that inherently guarantees correctness, or do you have human employees laboriously curate correctness? The sheer volume of C headers for a sophisticated modern microcontroller can be enormous, and if it’s not automatically-generated then you have only your example code — or worse, customers building their own products — to chase out corner cases where they’re incorrect.

Really these first three questions are closely interlinked: if you find that you can’t write your SDK against only publicly-attested headers, that should be a big hint that you’ve filtered-out too much: that your customers won’t be able to write their own code against those headers either.

4. Are your register definitions available in machine-readable form (e.g. SVD)?

ARM’s SVD (“System View Definition”) format was created as a machine-readable description of the register layout of Cortex-M-based microcontrollers, for the consumption of development tools — so that a debugger, for instance, could describe “a write to address 0x5800_0028” more helpfully as “a write to RTC.SSR”. But the utility of such a complete description is not limited to debuggers: in the embedded Rust ecosystem, the peripheral access crates or PACs that consist of the low-level register read and write functions which enable targetting each specific microcontroller — a sort of crowd-sourced SDK — are themselves generated straight from SVD files. (Higher-level abstractions are then layered on top by actual software engineers.)

Even for C/C++ codebases that are more directly compatible with the vendor’s SDK, it might sometimes be preferable to generate at least the low-level register-accessing code in-house rather than use the vendor SDK: for instance to generate test mocks of the register-access API — or, for code targetting a range of similar microcontrollers, separating common peripherals (e.g. the STM32 GPIO block, which is identical across a large number of different STM32 variants) from per-target peripherals where each chip variant needs its own code (e.g. the STM32 “RCC” clock-control block).

At Electric Imp we did both of those things: our range of internet-of-things products spanned several generations of (closely-related) microcontroller, with all of these products remaining in support and building in parallel from the same codebase for many years, and so we needed a better answer to Question 8 than our silicon vendor provided at the time. (In Rust terms, we needed something that looked like stm32-metapac, not stm32-rs.) And using the SVD files to generate test mocks of the register API, let us achieve good unit-test coverage even of the lowest-level device-driver code (a topic I hope to return to in a future blog post).

Basically, having a good SVD file available (perhaps itself generated from the chip source) gives your customers an “escape hatch” if they find their needs aren’t met by your published SDK. Although SVD was invented by ARM to assist takeup of their very successful Cortex-M line, it is so obviously useful that SVD files are becoming the standard way of programmatically defining the peripheral registers of non-ARM-based microcontrollers too.

5. Is your pinmux definition available in machine-readable form?

Most microcontrollers have far more peripherals on-board than they have pins available to dedicate to them, so several different peripherals or signals are multiplexed onto each physical pin; that way, customers interested in, say, UART-heavy designs and those interested in SPI-heavy designs, can all use the same microcontroller and just set the multiplex, the pinmux, appropriately to connect the desired peripherals with the available pins. Often, especially in low-pin-count packages, up to sixteen different functions can be selected on each pin.

This muxing information, like the register definitions themselves, is helpful metadata about the chip — and, thus, about software targetting it. A machine-readable version of this information can be used to make driver code more readable, and more amenable to linting or other automated checks for correctness.

Pinmux information in datasheets is typically organised into a big table where each row is a pin and the columns are the signals available; at the very least, having this mapping available as a CSV or similar would make it easy to invert it in order to allow the reverse lookup: for this signal, or this peripheral, what pins is it available on? Laboriously manually creating that inverse map was always one of the first tasks to be done whenever a new microcontroller crossed my path at Electric Imp.

6. Are all the output pins tristated when the chip is held in reset, and on cold boot?

Honestly this question is a specific diss-track of one particular microcontroller which failed to do this. I mean, lots of perfectly sensible microcontrollers tristate everything on cold boot except the JTAG or SWD pins, and that exception is completely reasonable. But this part drove a square wave out of some of its output pins while held in reset. It’s hard to fathom how that could have come about, without some pretty fundamental communication issues inside that chipmaker about what a reset signal even is and what it’s for.

(The microcontroller in question was part of a product line later sold-on to Cypress Semi; it might have been more fitting to sell it to Cypress Hill, as the whole thing was insane in the brain.)

7. Is it straightforward to get notifications when your public documentation changes (e.g. new datasheet revision released)?

I’ve said a few somewhat negative things about chipmakers so far, so here comes a solidly positive one: Renesas do this really well. Every time you download a datasheet or chip manual from their website, you get a popup offering to email you whenever a newer version is released of the document that you’ve just downloaded. Particularly at the start of a chip’s lifecycle, when these documents may include newly-discovered chip errata that require customer code changes, this service can be a huge time-saver. Chipmakers who don’t already do this should seriously consider it: it’s not rocket-science to implement, especially as compared to, say, the designing of a microcontroller.

8. Is it straightforward to use your SDK as a software component in a larger system?

The “developer experience” of a microcontroller SDK typically looks like: “Thank you for choosing the Arfle Barfle 786DX/4, please select how you’d like it configured, clickety-click, ta-da! here’s the source for a hello-world binary, now just fill in these blanks with your product’s actual functionality.” And so it should, of course, because that’s where every customer starts out. But it’s not where every customer ends up, especially in the case of a successful product (which, really, is the case you ought to be optimising for): there’s a bigger, more complex version 2 of the customer’s hardware, or there’s a newer generation of your microcontroller that’s faster or has more SRAM — or perhaps you’ve “won the socket” to replace your competitor’s microcontroller in a certain product, but the customer has a big wodge of their own existing hard-fought code that they’d like to bring across to the new version of that product.

In all of these cases, your SDK suddenly stopped being a framework with little bits of customer functionality slotted-in in well-documented places, and started being just one, somewhat replaceable, component in a larger framework organised by your customer. You don’t get to pick the source-tree layout. You probably don’t get to write main() (or the reset handler); if there’s things that need doing there for your specific chip to work, then see Question 1.

Being a software component in a larger system doesn’t preclude also having a friendly out-of-box developer experience; it just means that, when customers peer beneath the hood of the software framework that you’ve built them, what they see is that the core of the framework is a small collection of well-designed fundamental components — built with all the usual software-engineering values such as separation-of-concerns, composeability, unit-testing, and documentation — which can be used on their without the customer needing to understand your entire SDK.

When I worked at Sigmatel on their SDKs for microcontroller-DSPs for the portable media player market, it was clear that successful customers came in two types: there were the opportunist, low-engineering folks who took the SDK’s example media player firmware, stuck their own branding on it if that, and shipped the whole thing straight out again to their own end-users; and the established, high-engineering folks who already had generations of media-player firmware in-house, and just wanted the device drivers so that they could launch a new product with the familiar UI of their existing products. And there was nobody in-between these extremes, so it was not a good use of time to try to serve that middle-ground well.

In a sense the importance of a good answer to this question was emphasised by Finnish architect — that’s buildings architect, not software architect — Eliel Saarinen, who famously said “Always design a thing by considering it in its next larger context — a chair in a room, a room in a house, a house in an environment, an environment in a city plan.” (Quoted posthumously by his son, also an architect.)

I wish I’d seen that quote when I was just starting in software engineering. One of the most useful and widely-applicable tenets that I’ve been learning the hard way since, is this: Always keep in mind whether you’re designing a system, or whether you’re designing a component in a larger system. Hint: it’s never the former.

Enable and disable methods are an anti-pattern

Here's a C++ API for an abstracted I2C master peripheral, such as might be found on a microcontroller – an API which I invented:

class I2C
{
public:
    virtual ~I2C() {}

    virtual void setClockSpeed(ClockSpeed speed) = 0;
    virtual void enable() = 0;
    virtual void disable() = 0;
    virtual ssize_t read(uint8_t target, const uint8_t *subAddress,
                         size_t subAddressBytes, uint8_t *buffer,
                         size_t bufferBytes) = 0;
    virtual ssize_t write(uint8_t target, const uint8_t *message,
                          size_t messageLength) = 0;
};

The details of how an I2C bus actually works aren't so relevant to the current issue, so I've removed the comments. What's interesting here is what the calling code needs to look like: the caller has to set the desired clock-speed, enable the peripheral, read and/or write using it, and then disable it again.

This code has been happily chugging away inside Electric Imp devices of various types for about seven years now. Which is why it comes as such a surprise to me that I now think it's terrible.

The realisation came after spending some time learning the Rust language, whereby I've been trained into caring a lot about object lifetimes – because the compiler actually rejects code containing use-after-free errors and whole swathes of further bugs of that sort. I've seen bugs we diagnosed via in telemetry reports from crashing units in production, which would have been caught as compile-time errors in Rust. So Rust is great, once you've got your head round it; but in fact it's so great that I now find myself writing better C++ because of the experience of writing some Rust.

From an object-lifetime point-of-view, the API listed above is error-prone and so undesirable. It's too easy for client code to get things wrong: calling read() before you've called enable(), calling read() after you've called disable(), or calling setClockSpeed() – er, wait, when should you call setClockSpeed()? Is it only allowed before enable? Or is it only allowed afterwards? Or can it be in either position? What guarantees on that point are offered by implementations of this interface?

An act of API design should be an act of humility; you should, to a certain extent, be aiming to make other people's lives easier at the expense of making your own life harder. I failed to do that here. Offering a read() method that just returns ERR_NOT_ENABLED if you haven't called enable() yet, is just leading callers down the garden path. That ordering should be expressed in the API, so that, in the Rust style, it's a compile-time error to get it wrong (or alternatively, in another situation familiar to Rust programmers, so that the error simply isn't expressible in the first place).

And how can that be arranged? By not giving the caller access to the read() method until they've called the enable() method. What if we remove the read() and write() methods from class I2C altogether, and put them in a "handle" class which callers can't get hold of except by enabling the peripheral? This ends up looking like the API below:

class EnabledI2C
{
public:
    virtual ~EnabledI2C() {}

    virtual ssize_t read(uint8_t target, const uint8_t *subAddress,
                         size_t subAddressBytes, uint8_t *buffer,
                         size_t bufferBytes) = 0;
    virtual ssize_t write(uint8_t target, const uint8_t *message,
                          size_t messageLength) = 0;
};

class I2C
{
public:
    virtual ~I2C() {}

    virtual std::unique_ptr<EnabledI2C> open(ClockSpeed cs);
};

The disable() method has disappeared altogether; when the calling code has finished with its I2C controller, it can just reset or destroy the unique_ptr, and the destructor of EnabledI2C (or rather, the destructors of its subclasses) can do the work of disable(). Likewise, the work of enable() can be done in the constructor, bringing some RAII to the situation and making the implementations, as well as the calling code, less error-prone.

Without a method called "disable", "enable" looked like the wrong name, so it became "open". And this refactoring has also cleared up the ambiguity around setClockSpeed(); in fact, one implementation of this I2C interface allowed it to be called at any time, even in-between reads, but in another implementation it only took effect at enable() time. In practice, the calling code all expected it to work in the latter way, so we can codify that directly into the new API by incorporating it into the "enable" ("open") call itself, so that it's impossible to call it too early or too late.

And again but in C

C++'s support for RAII definitely adds both neatness and robustness to the code here. But part of the principle – that where API calls must be ordered or nested in order to work properly, that ordering or nesting should be enforced by the code rather than meekly whined-about by its documentation – applies even in a C version of the same API:

struct I2C; // opaque
struct EnabledI2C; // opaque

EnabledI2C *I2C_OpenEnabled(I2C *i2c, unsigned clockSpeed);

ssize_t EnabledI2C_Read(EnabledI2C *ei2c, int8_t target,
                        const uint8_t *subAddress,
                        size_t subAddressBytes, uint8_t *buffer,
                        size_t bufferBytes);
ssize_t EnabledI2C_Write(EnabledI2C *ei2c, uint8_t target,
                         const uint8_t *message, size_t messageLength);
void EnabledI2C_Close(EnabledI2C **pei2c);

Without RAII, the "close" corresponding to the "open" must be explicit, but at least it's still not possible for the client to call things in the wrong order.

And again but in Squirrel

This part is a bit of a cheat; the Electric Imp Squirrel API isn't actually implemented in Squirrel; rather, it's a set of bindings from Squirrel to the C++ code in the improm. But even so the same pattern can apply; here's (the native-Squirrel equivalent of) what it currently looks like:

class I2C {
    function configure(clockSpeed) { ... }
    function read(deviceAddress, registerAddress, numberOfBytes) { ... }
    function write(deviceAddress, registerPlusData) { ... }
    function disable() { ... }
};

This API is used by our customers left, right, and centre, and we can't ever change it. (We could extend it, perhaps.) But if we had our time over again, I'd have done it like this:

class I2C {
    function open(clockSpeed) { ... return EnabledI2C(...); }
};

class EnabledI2C {
    function read(deviceAddress, registerAddress, numberOfBytes) { ... }
    function write(deviceAddress, registerPlusData) { ... }
};

As in C++, the disable() call has disappeared, because it's all done in the destructor of the EnabledI2C object, which is called when the Squirrel reference-counting system determines that the object is no longer being used. The calling code might look like this:

local e = hardware.i2cAB.open(100000);
e.read(...);
e.write(...);
e = null;

Alternatively, it could just rely on e going out of scope at the end of the function, instead of explicitly setting it to null. But the cheating here has intensified: native Squirrel does not include destructors, and so doesn't allow for RAII. However, Squirrel objects that are bindings to other languages – such as the C++ bindings used by the Electric Imp API – can have destructors using Squirrel's sq_setreleasehook mechanism, and so can be used with RAII.

Portability, Or Cut-And-Paste?

Suppose you’ve got some functionality with two completely different implementations on different platforms:

// Event.h v1
class Event // Win32 waitable event
{

  void *m_hEvent;
public:
  Event() : m_hEvent(::CreateEvent()) {}

  void Wake();
  ...
};

versus

// Event.h v2
class Event // Posix waitable event
{
  int m_fd[2];
public:
  Event() { ::pipe(m_fd); } // or maybe eventfd on Linux
  void Wake();
  ...
};

And, by the way, if you’ve got a class where only some of the implementation is different on different platforms, you’ve first got an opportunity for refactoring to extract that part of the implementation as a self-contained class. Portability and maintainability are aided if each of your classes is either wholly portable, or wholly platform-specific. But as it is, you’ve got a more immediate problem, which is that there’s two classes both called Event, and any translation unit that includes both headers isn’t going to compile. Here’s one solution:

// Event.h v3
#ifdef WIN32
class Event
{
  ...
};
#else
class Event
{
  ...
};
#endif

This, plus its close relative

// Event.h v4
#ifdef WIN32
#include "win32/Event.h"
#else
#include "posix/Event.h"
#endif

could well be the most commonly-encountered solutions to this problem in real-world code. But they’re not without problems. One problem is with code analysis tools, including such things as production of automated documentation with Doxygen. Really you want such tools to analyse your whole codebase, and if they do they’ll get hopelessly confused with the two classes both called Event. (You can set up preprocessor defines in Doxygen so that it only sees one Event class -- but that only means the other doesn’t get documented, or that you have two entire sets of Doxygen output for the two platforms, neither of which sounds desirable.)

To solve this problem, you’ve going to have to give the classes different names:

// Event.h v5
class Win32Event
{
  ...
};

class PosixEvent
{
  ...
};

No, no, no, this is C++, there’s a built-in language facility especially for namespaces, you don’t have to reinvent it in the class names themselves:

// Event.h v6
namespace win32 {
class Event
{
  ...
};
} // namespace win32

namespace posix {
class Event
{
  ...
};
} // namespace posix

(Why the comments on the “}” end-of-namespace lines? Because the error messages you get from C++ when you accidentally miss out an end-of-namespace brace are often insane and impenetrable, especially if you do it in a header file, and it’s convenient to be able to grep namespace *.h *.cpp and check whether braces appear in matching pairs in the grep output.)

Notice that both declarations are parsed on every compilation on either platform. This might require you to move some method definitions (those calling such unportable APIs as ::CreateEvent and ::pipe) into a cpp file. But even compiling the other platform’s declarations helps ensure that changes on one platform won’t break the other platform without somebody noticing. And a Doxygen run, or other automated code analysis, will at least get to see both sets of declarations even if not both sets of definitions.

While going down the route of helping platform developers avoid breaking other platforms, it’s been conspicuous so far that there’s nothing in Event.h making sure that the Win32 and Posix implementations keep the same API. Without such enforced consistency, there’s a risk of client code unwittingly using non-portable APIs. The way to enforce the consistency, of course, is to derive from an abstract base class containing the API:

// Event.h v7
class EventAPI {
public:
  virtual ~EventAPI() {}
  virtual void Wake() = 0;
  ...
};

namespace win32 {
class Event: public EventAPI
{
  ...
};
} // namespace win32

namespace posix {
class Event: public EventAPI
{
  ...
};
} // namespace posix

This is, after all, exactly the sort of thing you’d do if you had two different sorts of Event that were both used in the same build of the program. Really the fact that no one individual binary will contain instances of both classes, doesn’t mean that the quest for well-factored design should be thrown out of the window and replaced with cut-and-paste.

Now, though, all the method calls have become virtual function calls. For most of any given codebase, that won’t matter, but there’ll always be hot paths and/or embedded systems where it does, and indeed an event class might quite plausibly be on such a critical path. And after all, compiling the client code itself provides a, potentially large, body of checks that the API has remained consistent over time. It’s reasonable to adopt the position that using design techniques to discourage unthinkingly changing core APIs in a non-portable way isn’t actually necessary, so long as the compilation-failure results of such a change are always speedily available from all target platforms, such as from an automated-build or continuous-integration system.

If, conversely, your class is only used in situations where nobody’s really counting individual machine cycles, you could hide the implementations altogether, at the cost of ruling out stack objects and member variables of type Event and requiring a heap (new) allocation every time one is created:

// Event.h v8
class Event {
public:
  virtual ~Event() {}
  virtual void Wake() = 0;
  ...
};

std::auto_ptr<Event> CreateEvent();

// Event.cpp
std::auto_ptr<Event> CreateEvent()
{
  ... return whichever derived class of Event is appropriate ...
}

But for now let’s assume you can’t really conceal the two platform-specific declarations, and go back to Event.h version 6 or version 7. Let’s look at how client code gets to pick which of the two implementations it uses. For a start, not like this:

class NominallyPortableDomainAbstraction
{
#ifdef WIN32
  win32::Event m_event;
#else
  posix::Event m_event;
#endif
  ...
  m_event.Wake();
  ...
};

Flouting our design rule that each class is either wholly portable or wholly platform-specific, this sort of thing couples portability decisions into all client code, which is very undesirable. This is much better:

// Event.h v9
... as before in v6 or v7

#ifdef WIN32
typedef win32::Event Event;
#else
typedef posix::Event Event;
#endif

// Client code
class NominallyPortableDomainAbstraction
{
  Event m_event;
  ...
  m_event.Wake();
  ...
};

It’s still not great, though: if there are lots of classes involved, lots of code ends up inside the #ifdefs. How can we minimise the amount of #ifdef’d code? Well, here’s one way:

// Event.h v10
... as before in v6 or v7

#ifdef WIN32
using namespace win32:
#else
using namespace posix;
#endif

But this, of course, introduces everything from those namespaces into the surrounding namespace. Not only is that namespace pollution, it’s potentially misleading for client code, as there may be Win32-specific classes or functions -- ones with no Posix equivalent, for use in Win32 situations only -- which are now accessible in portable code without the telltale win32:: prefix. (And, of course, vice versa for Posix-specific ones.) Really we want to explicitly enumerate which classes and APIs are intended to be portable. So we can do this:

// Event.h v11
... as before in v6 or v7

#ifdef WIN32
namespace platform = win32;
#else
namespace platform = posix;
#endif

using platform::Event;
using platform::PollThread;
...

The intention is that the using declarations list precisely those facilities which are available on all platforms, but with differing implementations on each. Non-portable classes or functions can stay in namespace win32 (or namespace posix) so that any use of them in client code immediately flags that code up as itself non-portable.

There is one problem with this neat solution, though: it doesn’t actually compile. Or rather, once you do the same thing in several different header files, you’ll find that multiple namespace X = Y; statements for the same namespace X aren’t allowed, even if Y is the same each time. So, sadly, you have to come up with a different name for “platform” each time (or centralise the using in one header file, causing potentially-undesirable widespread dependency on that file):

// Event.h v12
... as before in v6 or v7

#ifdef WIN32
namespace eventimpl = win32;
#else
namespace eventimpl = posix;
#endif

using eventimpl::Event;
using eventimpl::PollThread;
...

There’s only one remaining wrinkle, which is that you can’t forward-declare a class-name if that name only exists due to using. So if there are header files that traffic in Event* or Event& and could otherwise be satisfied with a forward declaration class Event; and avoid including Event.h, then you can’t use Event.h version 12. (Version 9 is also out, as you can’t forward-declare typedefs either.) The best you can do is probably this:

// Event.h v13
... as before in v6 or v7

#ifdef WIN32
namespace eventimpl = win32;
#else
namespace eventimpl = posix;
#endif

class Event: public eventimpl::Event {};
class PollThread: public eventimpl::PollThread {};
...

although naturally that only works as-written if the eventimpl base classes don’t have constructors other than the default constructor; if they did, you’d have to write forwarding constructors in each derived class, making the code a lot less neat.

This Code Is Completely Bogus

Here’s some rubbish code I wrote:

class Connection: public util::Stream
{
    ...

public:
    class Observer
    {
    public:
 virtual ~Observer() {}

 virtual unsigned OnHttpHeader(const std::string& key,
          const std::string& value) = 0;
 virtual unsigned OnHttpData() = 0;
 virtual void OnHttpDone(unsigned int error_code) = 0;
    };

private:
    explicit Connection(Observer*);
    friend class Client;

public:
    ~Connection();

    unsigned int OnActivity();

    // Being a Stream
    unsigned Read(void *buffer, size_t len, size_t *pread);
    unsigned Write(const void *, size_t, size_t*) { return EINVAL; }
};

typedef boost::intrusive_ptr<Connection> ConnectionPtr;

class Client
{
public:
    Client();

    /** Passing a NULL verb means POST (if !body.empty()) or GET (otherwise).
     */
    ConnectionPtr Connect(util::PollerInterface *poller,
     Connection::Observer *obs,
     const std::string& url,
     const std::string& extra_headers = std::string(),
     const std::string& body = std::string(),
     const char *verb = NULL);
};

It’s meant to be an HTTP client implementation, where the Connect() call returns a smart pointer to a connection object that manages its own lifetime. The Connect() call sets it all off, and attaches it to the PollerInterface object, which calls back into the Connection object (the OnActivity method) whenever the socket becomes writable (or readable, depending on where we are in the http transaction).

I mean, just look at it: it’s obviously completely bogus. [Short pause whilst you just look at it.]

Actually, no. It wasn’t at all obvious to me just from looking at it, that it was completely bogus. The only point, in fact, at which it became obvious that it was completely bogus, was when it started causing bugs.

And the bugs it caused were quite awkward ones: crashes (and Valgrind violations), obviously timing-related, to do somehow with Connection pointers still being used after the object had gone away — which should really have been disallowed by the smart-pointer class.

It turned out that the problem was if the transaction completed too quickly. At the time the code was written, the PollerInterface object didn’t own (smart pointers to) the pollable items hung off it. So, in order to stop the Connection object disappearing, due to the final reference going away, during a call to OnActivity, OnActivity itself creates and holds an additional reference to the Connection object during its execution. But if the transaction got started quickly enough, the first call to OnActivity would happen before the constructor returned — in other words, before the pointer had been assigned to the smart-pointer that’s the result of Connect(). So the “additional” reference held inside OnActivity would be the only reference — and when it went away at the end of the function, there’d be no outstanding references and the object would be deleted.

The effect was as if the constructor had included “delete this” — resulting in the new call in “p = new Connection” returning a pointer that was already dead and dangling before even being assigned to p.

Completely bogus. And worse, a completely bogus design; given the constraints, there was nothing that could possibly be done in the methods of the classes Client and Connection that would correctly implement that interface. The interface itself needed to change. Fortunately, I decided it didn’t need to change much:

class Connection
{
    ...
public:

    /** Start the HTTP transaction.
     *
     * Immediate errors (failure to parse host, failure of connect()
     * call) are returned here; errors happening any later come back
     * through Observer::OnHttpDone. In fact, OnHttpDone may be called
     * before Init() returns, i.e. you need to be ready for OnHttpDone
     * calls before you call Init(). If Init() returns a failure,
     * OnHttpDone has not been called, and is guaranteed not to be
     * called afterwards. Otherwise, it's guaranteed it WILL be
     * called.
     */
    unsigned int Init();
    ...
};

So you get a completely inert ConnectionPtr back from Client::Connect, and only then — once you’ve safely squirreled-away the smart-pointer — do you light the blue touch-paper by calling Connection::Init().

But although this version at least makes it possible to use the functionality correctly, it doesn’t really make it easy — and that should be the real goal of any act of API design. There’s that uneasy division of labour between Connect() and Init() — methods on two different classes — and there’s a whole paragraph of complex object-lifetime issues to read and understand (or, as they’re otherwise known, bugs waiting to happen) for users of Init().

This, and particularly the lifetime of the Connection::Observer object, which usually ends up having to call “delete this” in its OnHttpDone() method, left me with one of those code itches that tells me that I (and, the largely theoretical, other users of this API) am writing more complex and icky client code than I should be.

Neatening this up required making the object-lifetime issues more sane, which in turn involved greater use of smart pointers. (Not quite Wheeler’s Law, because there was already a use of indirection, and the change involved only a strengthening from reference to ownership.) In the next release of Chorale, the PollerInterface has been replaced by a Scheduler, which keeps smart pointers to its pollable items, allowing the HTTP client API to be simplified to this:

class Connection: public util::Stream
{
public:
    virtual ~Connection() {}

    /** Called with data from the returned HTTP body (if transaction
     * succeeds).
     */
    virtual unsigned Write(const void *buffer, size_t len, size_t *pwrote);

    /** Called with each incoming HTTP header (if connection succeeds).
     */
    virtual void OnHeader(const std::string& /*key*/, 
     const std::string& /*value*/) {}

    /** Called with the overall result of the connection/transaction attempt.
     */
    virtual void OnDone(unsigned int error_code) = 0;
};

typedef util::CountedPointer<Connection> ConnectionPtr;

class Client
{
    class Task;

public:
    Client();

    /** Passing a NULL verb means POST (if body != NULL) or GET (otherwise).
     */
    unsigned int Connect(util::Scheduler *poller,
    ConnectionPtr target,
    const std::string& url,
    const std::string& extra_headers = std::string(),
    const std::string& body = std::string(),
    const char *verb = NULL);
};

So the Scheduler owns the (unseen to the library user) connection Task objects, and the connection Task objects own the Connection stream target objects. Connect() can return immediate errors from the socket ::connect call or from URL parsing, while deferring any other errors to come back through a later OnDone callback — all without there being any ambiguity of the lifetime of the streams or their observers.

(There would be a problem if the Connection object also had a smart pointer to the Task object, as then neither would ever be deleted and both would become memory leaks. But, because the data is pushed from the Task to the Connection, the Connection never needs to see the Task object — and indeed can’t, because Tasks live and die entirely inside library code, and users of the library can’t even obtain pointers to them.)

Simplicity does not precede complexity, but follows it —attributed to Alan Perlis

So here’s the thing. The third design is clearly better than the second. Well, it’s clearly better than the first, too, but mainly because the first doesn’t work, which is a boring and trivial way for one design to be better than another. The interesting thing is that it’s better than the second.

And better it certainly is — making this change halved the size of the code that uses the library, as well as making it more intentional and less fragile and stylised.

So why, having got to the second design, was I not satisfied? Why did I carry on thinking about it, waiting for the inspiration of the third design to strike? And why, having come up with the third design, was there a feeling of happiness that wasn’t present when writing the the second one, even when it passed all the unit tests the first one failed?

The only answer I can come up with is to theorise the existence of an almost aesthetic sense of code quality — which is worrying in a couple of ways. Firstly, because what is instinctive is rarely communicable, and what is not communicable is soon lost: a software engineer’s fate is that of the wheelwright in the old story of Duke Huan.

But worse than that: if code quality is in fact an aesthetic, and thus extrarational, experience, then it raises the prospect that others, even other good software engineers, could have a different sense of aesthetics, ultimately resulting in a point where you and they are pulling the same code in opposite directions. (I heard recently of a software organisation, believers in the currently-fashionable “agile”, “refactor-mercilessly” style of development, in which two otherwise talented engineers spent all their time rewriting each others’ code rather than pushing things forward — as their aesthetic senses, and frankly their assumptions about who was “in charge” in the deliberately un-micro-managed environment, butted heads.)

No aesthete could get away with “correcting” the second design above into the first: the failing unit tests would prevent that. But are there those who would correct the third design into the second, in the opposite direction to me? If so, why? And, even more importantly, if not, why not?

Four Quarters Of The Codebase

As a software project grows, eventually it becomes unwieldy to keep the entire source in one single gigantic folder: you’ll need to split it up. Canonically, and indeed quite correctly, the first idea most architects then have is to split the source into a main program plus one or more libraries that provide ancillary services.

Once that’s done, everyone on the project who sits down to write some new code, will have to think at least for a moment about where to put the source file. The answer, of course, is that the new code should go in the main program if it’s fairly specific to the current project (or if it depends on code that’s already in there), and into the library otherwise. The “force of gravity” — developers’ sense of where code should go unless there’s good reason otherwise — should be in the direction of pulling functionality over into the generic code.

Sometimes this decision seems harder than it should be: if that’s the case, what’s often going on is that the functionality in question should really be split up — with a generic part in the library, and the specific use this project makes of it in the main program. In this way, source code organisation can act as a forcing function for good design.

If that’s so useful, can we get extra design-forcing out of how we organise the source? One issue that often throws a spanner in the works of software designs whose creators didn’t originally bear it in mind, is portability between software platforms. It would be decidedly helpful if the source code organisation, and/or the system architecture it (one hopes) reflects, could help prevent platform specialists from unthinkingly tying code to a particular platform when in fact it’ll be needed more widely in future.

So instead of just application versus library, it’s helpful to organise the codebase so that a developer sitting down to write new code, will have to choose between four different places to put it:

Application-specific, platform-specific	Library code, platform-specific
Application-specific, platform-agnostic	Library code, platform-agnostic

This means that, if you’re writing an application that you’re targetting at, say, both Iphone and Windows, or both desktop and embedded, then the actual application logic needs to go in the south-west corner, where implementations on any platform can use it; user-interface code mostly goes in the north-east, where other applications wanting similar UI components can use it; and only the minimum of code necessary to wire the two together goes in the north-west.

The force of gravity in this diagram acts towards the south and the east: if you can turn code from a Win32 application into generic Win32 components, or into a portable implementation of the logic, that’s a design improvement. If you can split the code up, even split a single class up, along those lines, that’s an improvement too. And by forcing developers to make the four-way choice before they write a source file, you’re helping everyone in the team to think about these issues and to factor the code in the best way as it’s first written. Fostering a feeling of “design guilt”, in yourself or others, when placing functionality further north or west than it belongs, gives everyone an easily-grasped metric for well-factored designs.

Really this just another way of thinking about the model-view-controller pattern; the mapping is straightforward, except that MVC doesn’t have a word for the fourth quarter, libraries which are assumed to be at a lower level than the components MVC deals with:

Controllers	Views
Models	(Libraries)

but, even so, embodying MVC in the source layout itself means that its issues get thought about earlier in the software process.

Now naturally, any real codebase above a certain size won’t consist of just “the” application and “the” library. There’ll be a whole collection of libraries, and perhaps of applications too, arranged in a dependency graph — hopefully, for reasons Lakos expounds, a graph which forms a non-cyclic hierarchy. But it should be possible to partition this graph into (up to) four parts, matching the four quarters of the codebase, and with all dependency arrows pointing southwards, eastwards, or south-eastwards. No view or model should depend on a controller; no platform-agnostic library should depend on any view, model or controller. Again, these are truisms of MVC design, but thinking about them in their four quarters gives an easy visual way of thinking about and discussing the issues.

Incidentally, my own Chorale codebase currently (as of v0.15) fails this test, or at least gets a technical pass only: it doesn’t have any platform-specific library code or directories, as all that code is rather bogusly stuffed in the application-specific, platform-specific directories. (Some classes are, in fact, platform-specific library classes, but the split isn’t enforced in the source organisation, either by libraries or by directories.)