January 4th, 2010 | Tags: , , ,

I’ve encountered a faulty assumption about how TCP works, actually in multiple code bases I’ve maintained. Interestingly, my colleague Jeff and perhaps soon-to-be colleague Dane have also encountered this more than once and have dubbed it “Super High-Performance Networking Code.”

In Super High-Performance Networking Code, the sending side might look something like this:

    void OnFooEvent(Event e)
    {
        char *ptr = e.GetVariableLengthPayloadDataAsBytes();
        int len = e.GetLengthOfPayloadData();
        int rc = send(m_socket, ptr, len, 0);
        // handle error.
    }

… and the receiving side might look like this …

    void ReceiveLoop()
    {
        while (!m_bDone)
        {
            char buf[REALLY_BIG];
            int len = recv(m_socket, buf, sizeof(buf), 0);
            if (len > 0)
            {
                HandleExactlyOneFooEventWithVariableLengthData(buf, len);
            }
        }
    }

The fatal assumption is that read sizes on the receiving end are coincident with the write sizes on the sending end.

TCP networking is a stream-based protocol and it makes NO guarantees that the read and write sizes are coincident. In fact, it has a few mechanisms which will actively thwart this, including Nagle’s algorithm. Also, all TCP networks have an MTU, and packets larger than this will be sliced up for delivery (although without affecting the data contained within or the guaranteed order of delivery). There is some handshaking to discover the minimum MTU along a route, though this is intended to allow client systems to adapt for maximum throughput and is not a guarantee that slicing won’t happen. It’s more like a guarantee that slicing won’t happen frequently. Additionally, loaded routers may well choose to split transmit units or collect and concatenate them in order to meet quality of service and throughput requirements.

Unfortunately, though, making this assumption will work often enough with small packet sizes to lull some developers into believing they’ve confirmed that assuming read and write sizes are coincident is sound.

If you are making this assumption, you need to redesign your wire protocol to include within it a mechanism for delineation of the data, such as sending the data size up front, or using some sort of end-of-data marker. You then need to collect the reads on the receiving side until you’ve reached the demarcation or expected data size.

December 30th, 2009 | Tags:

It’s been a little while since I wrote Comments Unhelpful, and two recent events bring me back to the topic. The first event is discussion brought up by a colleague about implementing commenting requirements in our coding standard. The second event is Mark Schumann’s recent post including explanation of what would have helped him crack the codes of a physicist.

Since I’ve ranted about what doesn’t work, perhaps I should post what I’ve found that does work.

Rule One: Prefer “Rename Class,” Rename Method, and Extract Method Over Comments

The code itself is durable. Comments are not. If you can obsolete the need for a comment by renaming a class or method, do it.

A note on Extract Method. If you haven’t read Fowler’s Refactoring, this was a particularly enlightening “Aha!” for me.

When you are writing a function, and those little one-line comments come up that explain the next little logical block of code, you should extract that little block of code into a method, name it so that it conveys the information from the comment, and kill the comment. This is good design as well as good documentation.

Rule Two: Don’t Put Anything in a Comment which has Already Been Expressed

This simply means that you should apply DRY to comments like you would apply it to any other piece of the system. Many of my points in Comments Unhelpful are directly related to this rule.

Do not include any part of your function or class signature, such as return type, parameter types, exception names – and certainly don’t repeat the function or class’s name. The only exception is referring to an item to add more documentation. Also, do not feel compelled to add documentation for every parameter if you have no non-obvious information to add.

Do not include any information available from version control, such as author, modification date, or revision number. Version control is much better at tracking this information than you are.

Rule Three: Class and Method Comments Should Contain a One-Sentence Summary

This is not an argument for adding a comment to every method and class, but every method and class which has a comment should start with a one-sentence summary. This enables programmers to understand the code more quickly, as well as supports tools like Javadoc and Doxygen.

The one-sentence summary should clearly document the item’s single responsibility. I can’t find reference to it now for proper attribution, but I once read that every class in the system should be describable in one sentence, and the sentence should always be simple (as opposed to compound) and have no qualifying clauses. This works to expose design problems with new and refactored code. You may not be able to apply this if you are documenting existing code without refactoring.

Use first-person active voice to help clarify your thoughts. For example, “I sort a list into an order suitable for presentation.” instead of “Sort a list for presentation” or “Sorts a list for presentation” or “This method sorts a list for presentation.” This is a sort of a commenting-by-rote jamming mechanism. I don’t know why, but personifying classes and methods seems to reify them better. I first encountered this in emacs lisp code.

Rule Four: Synopsis Over Exposition

“Synopsis” is the contribution of perldoc to my commenting rules. Exposition is hard to write, hard to read, and harder to maintain. A short synopsis of how the class or method is intended to be used is much easier to read and understand. You can add comments to your synopsis to clarify non-obvious points. You also receive the additional benefit that when you rename a method or class with a text search-and-replace, it does not invalidate your synopsis.

For example:

/**
 * I manage a pool of devices.
 *
 * Synopsis:
 *   DeviceContainer* p = new DeviceContainer();
 *   p->AddDevice("COM1:");
 *   p->AddDevice("COM2:");
 *   if (!p->Check()) fail("Don't have all devices!");    
 */
class DeviceContainer {

Rule Five: Why, What, and Not How

This is the “Linus Torvalds” rule. From the Linux kernel CodingStyle document:

Comments are good, but there is also a danger of over-commenting. NEVER try to explain HOW your code works in a comment: it’s much better to write the code so that the _working_ is obvious, and it’s a waste of time to explain badly written code.

Generally, you want your comments to tell WHAT your code does, not HOW. Also, try to avoid putting comments inside a function body: if the function is so complex that you need to separately comment parts of it, you should probably go back to chapter 6 for a while. You can make small comments to note or warn about something particularly clever (or ugly), but try to avoid excess. Instead, put the comments at the head of the function, telling people what it does, and possibly WHY it does it.

Rule Six: Astonishment

Anything which would astonish a programmer attempting to use the entity should be documented if it can’t be corrected.

December 28th, 2009 | Tags: , ,

In the past, I’ve reached for the C stdio library when I need to do binary file I/O in C++. Why? Well, much of the C++ iostreams interface is designed for character- rather than byte-based I/O:

  • I/O is generally done through the << and >> operators, which format types to a character representation.
  • Stream objects can be “imbued” with C++ locale objects, affecting various aspects of I/O, including how numbers are formatted and how bytes are translated into characters.
  • The ios::binary flag only affects line-ending conversion, not character conversion.
  • Stream objects’ locales are inherited from the global locale.

I figured this would make binary I/O difficult, but I never researched it.

The opportunity to figure this out has just come up. I wrote a class which builds a tar file on the fly by writing it to a generic iterator. Its AddFile() also reads the contents of the file to store from an iterator.

I wrote this under test without hitting the disk, but the last step is to supply binary file input and output iterators. After successfully implementing a binary file input iterator, I thought, “It’s hard to believe that Boost doesn’t already have these.” I posted on the list and quickly received an answer:

Use the standard streambuf iterators, istreambuf_iterator and ostreambuf_iterator.

Can we do that? What about locales and character set translation?

So finally I looked into this locale thing. It seems the ISO C++ specification guarantees that codecvt<>, which is the facet of the locale that translates characters, performs no translation in the case of converting from a narrow char buffer to a narrow char buffer. The specification considers this a “degenerate case.”

Conclusion: The C++ language specification guarantees that narrow iostreams will pass through untranslated bytes regardless of the current locale.

December 21st, 2009 | Tags: , , , , ,

The issue with Singletons is twofold. First, it is not a problem that you’d like to enforce only one instance of something within a system. It is a problem that you are enforcing only one instance of something within a system using the Singleton pattern. Why?

1. You are coupling every client of the class with the knowledge the class has only one instance. This comes from C++’s distinct syntax for obtaining the instance of a Singleton. This (a) very much violates the intent of encapsulation in object-oriented systems, (b) is a violation of DRY, and (c) is a violation of the Uniform Access Principle (or a slight generalization of it in any case).

2. In order to unit test effectively, you need to exercise different states. With some common usages of singletons, not all interesting states are reachable. Even the states which are reachable are more difficult to reach.

To give an example, I maintain a Java application which uses Singleton pattern for its configuration data object. You might think this is reasonable, since an application can have only one configuration, and that is an essential property of the system’s configuration, correct? The object’s getInstance() method deserializes the configuration object from some XML on disk on the first invocation, and reuses the object on subsequent invocations.

Now, how do I unit test this system? Let’s say I have some configurable object that can exhibit two kinds of behavior. I can write the XML in our testing directory one way, and test one kind of behavior, or I can write it the other way and test the other kind of behavior. Alternately, I can add a reset() or setInstance() to the Singleton, making it no longer a Singleton, and write machinery which modifies the config then essentially reboots the system. This is a lot of extra work. If you pay careful attention to what you’ve just done, you’ve replaced Singleton with something that I am now dubbing “Service Injection by Global Variable.”

It gets worse: Even for classes in the system where I can inject parameters, the parameters themselves, or far removed dependencies of them or the object, may depend on other global mutable state. If I have a system where Singletons are common, this erodes my confidence that my unit tests guarantee correct execution of my program. This is just the same old “global variables” argument – it is more difficult to reason about programs because you need to know not just the interface of an object and its liftetime, but also the global mutable state on which it depends to predict its behavior.

Solutions:

1. There are dependency-injection frameworks (e.g. Spring, in Java-land) that allow you to declare singleton instances in a way such that the same instance is injected to every object that requests the instance. This is not a problem, even though the word “singleton” appears: (a) the client code is not coupled with the knowledge that such an object is singleton, and (b) the knowledge that an object is singleton is expressed in a single place, and (c) the client code is identical regardless of whether the object is a singleton.

2. When you aren’t using a dependency-injection framework, injection (either by constructor or setter) is still the “right” route. I know that it seems hard, but there are plenty of tricks to make it easier, especially in C++ – the simplest of which is aggregating the most commonly used of the system’s services into a parameter object. In addition to what I’ve said above, this is the “right” choice because the application’s application object (for example) rather than the class providing some service should be the single point of knowledge deciding whether some service should be singleton across the application.

December 17th, 2009 | Tags: ,

I’ve been using git as my Subversion client at work lately. I’ve promised myself that I will blog more about this, but I still need to work out some kinks in my work flow and bootstrapping the process.

In any case, git bisect has turned out to be an invaluable tool for isolating the cause of reproducible failures when you are starting with a known good build and a known bad build. I’ve isolated three failures in the last couple weeks. All were subtle race condition or other threading issues which appear apparently at random – the worst kind!

Prior to this, we were doing what my coworker Jeff Messner calls “debugging by epiphany” – stare at the logs, then the code, then the logs, then the code, then the logs… until in a Zen koan moment you are enlightened.

Perhaps this is why the Linux kernel is so reliable. Linus advocating the identification of failures by bisection since the very first kernel documentation I ever read. Curiously, Linus himself authored the “git bisect” command.

In any case, I encountered a scenario yesterday where a recent build of a component resolved a customer’s issue; however, it wasn’t obvious which change was the actual resolution. I’m documenting this information these days in machine-readable format so that our program which builds customer installs will flag bad combinations of components before we ship to QA.

I tried to reverse the “good” and “bad” patches with bisect, hoping it would be smart enough to deduce that I was looking for the patch which introduced the fixing change, rather than the usual breaking change. It was no go – git tells me there is a problem with the patch ancestry; it absolutely requires the good change to appear before the bad.

It turns out that it wasn’t too difficult to just tell git that the good builds were bad and the bad builds were good in order to isolate the change which resolved the customer issue.

December 2nd, 2009 | Tags: , , ,

“Conventional xUnit wisdom” holds that one should organize tests into groups and implement these tests in one class, factoring common set up and tear down operations into the class’s setUp() and tearDown() methods. I think this wisdom is perpetuated mostly by the apparently intentional design of xUnit testing frameworks for this kind of usage. (Note that Boost.Test is fundamentally different, and people absolutely hate it when they first encounter it just because of this, even though it seems to work better in the long run for reasons we shall shortly see.)

This pattern quickly becomes problematic. For the first few quick hack tests, it works wonderfully. As the code under test evolves, you’ll usually add small batches of one, two or three tests each.

As you add the sixth test to a class, you notice that you did a pretty similar kind of set up for two tests. You can’t move this code into setUp(), since the other tests in the suite have the opposite precondition. So you factor this code out into a separate private method and call it in both places.

As you write your tenth test, you notice that checking the result from a particular method under test requires three repeated lines of code for each test. Being a diligent coder, you factor this out into an assertFoo() method and replace all the existing instances of the repetition.

As the design evolves, your class under test becomes one of two, then three, different strategies for dealing with a problem. As you start your third test suite, and start retyping the suite of methods you’ve written for testing this sort of class, you think, “This is getting crazy – I need to reuse those methods.” At this point, the first thing you think is, “Let’s create a superclass for this set of test cases, and pull up all these methods we’ve written so they can be reused.

Of course, this is usually a bad design practice, even outside of our xUnit test suites. But it is still a common one, because aggregation is just so darned painful in most common, modern, production-ready languages. In terms of design and reuse, it’s all downhill from here.

Why do we keep getting ourselves into this trap?

xUnit frameworks enforce the use of classes for structural grouping

I’m sure we probably all remember that “neato!” feeling we had when we first saw an xUnit suite and realized that the framework was utilizing reflection or some other language trick to run all the tests. “Hey, that’s pretty cool, and it saves a lot of work!” Well, sure it does.

It may not dawn on us right away that this is not what classes are for!

Normal object oriented design principles suggest that classes are for encapsulation and code reuse

…not for structural grouping of similar objects (in this case, tests), or to provide mechanisms to run lists of objects! We could say that our xUnit suite runners are a historic record of some shortcomings of Smalltalk and Java.

These two uses of classes are at cross-purposes

Showing that these two design patterns are not at cross-purposes would require showing that any logical structural grouping of tests would coincide with (or could be made to coincide with) useful patterns of encapsulation. This seems like a proof too difficult to attempt; however, I can say that my own experience suggests that logical structural grouping of tests does not coincide with useful patterns of encapsulation.

The xUnit Design Heuristic

Stated formally:

Since xUnit’s use of classes to group tests is in conflict with the use of classes for encapsulation and reuse in object oriented design, regard the use of test suites for reusable code or encapsulation as a code smell and prefer creating a driver class.

The driver class encapsulates all or part of a test run and serves as a target for Move Method for those assertions and helper methods. These classes should follow a number of design principles:

Make the driver class own all references to the class under test, fake and mock objects, and other state needed to run the test.

This is simply restating that our driver class’s purpose is encapsulation.

Defer construction of the test’s state to the last possible moment

For our tests to be thorough, they need to be able to test our class under test under many different preconditions. Setting up a lot of state in the class’s constructor may make it difficult or time-consuming to override later. Consider making the various methods set flags or other fields indicating how the test should be run, and having a runner method (or even method object) use these flags for running the test.

Use Builder Pattern to construct test specifications

If designed well, this can greatly improve the readability and the re-usability of the testing code:

    void test_Does_not_indicate_balance_receptive_if_balance_return_flag_is_N()
    {
        BasicRequest()
            .With(FID_CARDID).Of("MAS")
            .With(FID_BALANCE_RETURN_CAPABLE).Of("N")
            .SendsMessageWhere()
                .SPIField().Substring(1,1).Equals("N");
    }
November 29th, 2009 | Tags: , ,

I’ve just started ehnet-spc, a new Scala project on github for plotting Statistical Process Control (SPC) charts. It’s very bare-bones, but it can produce SVG charts. It only does subgroup size 1 with moving average ranges at the moment.

Here is a sample control chart.

November 22nd, 2009 | Tags: , ,

I’ve demoed my custom vim scripts for quite a few people in the past, and a few vimmers have shown interest. It’s not been until now that I’ve cleaned up the ball of Ruby code in my vimrc and separated it from the other stuff in there and made it public.

You can download EhHack 0.1.

In this release:

  • Ruby code was moved into .rb files
  • New per-language pluggable mechanism for compiling and running quick hacks
  • Support for Scala quick hacks added (already had support for C++, Java, and Haskell quick hacks)

More features should be coming soon. EhHack automatically adds C++ #include directives for standard library headers as you type. I would like to generalize this for Java, Scala, and Haskell imports.

Enjoy, and happing vimming!

September 25th, 2009 | Tags:

I work in a place driven by e-mail. Perhaps unhealthily so. Today, I’ve sent 18 e-mails and received 56 e-mails. Note that I’ve had my e-mail closed for several hours today, just to get work done. And so, I propose some new Outlook features (yes, I have to use Outlook) that would make me MUCH, MUCH more productive. They are:

Unsubscribe From E-Mail Thread

If I click this button, any person who uses the “Reply to All” button on any message in the thread will not automatically reply back to me. The “To:” and “Cc:” lines will not contain my address.

If someone actively adds me back to the To: or Cc: line, that’s fine. Perhaps I need to be involved again.

Wrong Person

This button works much like “Reply to All” button, except that:

  1. It contains a “Right Person” header, along with the “To” and “Cc” headers. When the message is sent, the “right person” is added to the “To” line.
  2. The text, “For this kind of issue, please contact <right person>.” is inserted in the reply body, and the cursor will be placed right after the period. If the “right person” is already copied on the e-mail, it will instead insert the text, “Please don’t copy me on this kind of issue – <right person> can handle this and bring me in, if necessary.”
  3. Hitting “Send” will also unsubscribe me from the e-mail thread as above.
  4. Outlook should track the total number of times a particular “right person” has been recommended to the message’s sender, and use increasing font sizes and increasingly gaudy colors for each successive e-mail recommendation.

Attach This E-Mail To Our Stupid Internal Tracking System

Clicking this button will prompt for a tracking number, and all future e-mails in the thread will be stripped of previously included content and appended to our obligatory Stupid Internal Tracking System’s tracking item (task, project, Excel page, whatever) where it can be publicly read.

Forward Answer Again

This button forwards an e-mail from your Sent Messages folder to the person it was originally sent to, as well as increase both the font size and the gaudiness of the font’s color, and copies the person’s manager.

September 5th, 2009 | Tags:

Someone recently presented this program on a forum:

int main() {
    signed char x = 0xaa;
    printf("%x", x);
    return 0;
}

They asked why this program produces the output “ffffffaa”, but if they changed the type of ‘x’ to an unsigned char, the program produces the output “aa”.

Here is my answer:

  1. The two’s complement system used by probably all of today’s systems to represent negative numbers represents negative numbers as if they were subtracted from zero, except that they “wrap around”. Therefore, -1 in two’s complement (assuming a 32-bit integer) is 0xffffffff, since it is 0 minus 1, wrapping around to the highest bit pattern of the data type. In fact, the reason two’s complement is generally used is that it makes addition and subtraction have no special cases, unlike one’s complement or using a separate sign bit.
  2. The sign is not shown in hexadecimal, typically. Therefore 0×1 is 1, and 0xffffffff is a 32-bit -1 (as opposed to writing -0×1), and 0xff is an 8-bit -1. Note that an 8-bit -1 (0xff) is the same thing as a 32-bit signed 255 (also 0xff) – you need to know the size of the type to interpret negative numbers.
  3. Because an 8-bit -1 is the same thing as a 32-bit 255, when the compiler needs to “promote” a value to a larger data type, it can’t simply copy the low bits to the larger data type’s low bits and zero the upper bits, because this would actually change the value! Instead, it must use sign extension. Therefore, when the compiler widens a signed integer which has the high bit set, it fills the extra high bits with 1s, and when it widens a signed integer which has the high bit clear, it fills the extra high bits with 0s.
  4. Widening from an unsigned type is easier – the compiler always zero-fills the extra high bits. Without signed-ness, there is no problem, since an unsigned 8-bit 0xff is the same value as an unsigned 32-bit 0xff.
  5. C and C++ have rules for promoting arguments to functions. Arguments smaller than the machine’s word size (which is int) should be promoted to int before passing to a function. So any function which takes a char or a short, is actually being passed an int, even though the function body itself will operate on it as a char or short.
  6. printf() doesn’t actually know the type of the arguments – it deduces them from the format string. Formats like “%x” and “%d” expect an int, so they extract an “int” from the stack. Since narrower integral and character types are promoted to int before being passed, you can use format specifiers like “%d” with char and short types, and they will usually work as expected.

This all means that the generated machine code for the program does something like this:

1. load a value 0xaa into an 8-bit register
2. sign extend 0xaa into a 32-bit register, making it 0xffffffaa in the new register.
3. push a pointer to the format string “%x” onto the stack
4. push the 0xffffffaa onto the stack
5. call printf
6. pop printf’s parameters off the stack

TOP