.\Components/CommComponents/FDMSInterleaveTcpipComm/Test_FDMSInterleaveTcpipComm.h(106) : fatal error C1001: INTERNAL COMPILER ERROR
        (compiler file 'msc1.cpp', line 1794)
         Please choose the Technical Support command on the Visual C++
         Help menu, or open the Technical Support help file for more information

was caused by:

    template<typename VersionPolicy> class SVDotPacketProductionPolicy;
    class SVDotPacketProductionPolicy<SV24VersionPolicy>;
    friend class SVDotPacketProductionPolicy<SV24VersionPolicy>;
    class SVDotPacketProductionPolicy<SV40VersionPolicy>;
    friend class SVDotPacketProductionPolicy<SV40VersionPolicy>;

Include\boost/bind/arg.hpp(25) : fatal error C1001: INTERNAL COMPILER ERROR
        (compiler file 'msc1.cpp', line 1794)
         Please choose the Technical Support command on the Visual C++
         Help menu, or open the Technical Support help file for more information

This one started after I added

#include <boost/bind.hpp>

to a file and a simple, every-day use of boost::bind(). Moving the #include directive up before a number of other standard includes resolved the issue.

C:\projects\DASV2>scons
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
cl /FoComponents\AuthComponents\Credit\CCMps\Release\Tests.obj /c Components\AuthComponents\Credit\CCMps\Release\Tests.cpp /nologo /
TP /nologo /MT /W2 /GX /GR /O2 /Zi /Zm200 /D "WIN32" /D "_WINDOWS" /D "NDEBUG" /D "_MBCS" /FD /D "_WIN32_WINNT=0x0400" /D "_WIN32_DC
OM" /IWindowsSDK\Include /IInclude /Icryptlib /IXML\Xerces\src /I. /IComponents\AuthComponents\Credit\CCMps
Tests.cpp
Include\boost/function/function_template.hpp(514) : fatal error C1001: INTERNAL COMPILER ERROR
        (compiler file 'msc1.cpp', line 1794)
         Please choose the Technical Support command on the Visual C++
         Help menu, or open the Technical Support help file for more information
scons: *** [Components\AuthComponents\Credit\CCMps\Release\Tests.obj] Error 2
scons: building terminated because of errors.

06/01/2009 – This one was the result of passing a pointer to a static member function to boost::function1<>.

C:\projects\DASV2>scons
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
python.exe Utilities/CxxTest/cxxtestgen.py --runner=XmlPrinter --have-eh --have-std --check-memory -o CBase\ISOMsg\Debug\Tests.cpp CBase\ISOMsg\Test_Fields.h
cl /FoCBase\ISOMsg\Debug\Tests.obj /c CBase\ISOMsg\Debug\Tests.cpp /nologo /TP /nologo /MTd /Od /W2 /GX /GR /Zi /Zm200 /D "WIN32" /D "_WINDOWS" /D "_MBCS" /FD /D "_WIN32_WINNT=0x0400" /D "_WIN32_DCOM" /D "DEBUG" /D "_DEBUG" /IWindowsSDK\Include /IInclude /Icryptlib /IXML\Xerces\src /I. /ICBase\ISOMsg
Tests.cpp
.\CBase/ISOMsg/Test_Fields.h(157) : fatal error C1001: INTERNAL COMPILER ERROR
        (compiler file 'msc1.cpp', line 1794)
         Please choose the Technical Support command on the Visual C++
         Help menu, or open the Technical Support help file for more information
scons: *** [CBase\ISOMsg\Debug\Tests.obj] Error 2
scons: building terminated because of errors.

06/18/2009 – This one is the result of attempting to use a template member of template class. Here’s the (abbreviated) code:

template<typename C, C v>
struct ThreeArgConstructor
{
    template<class BaseT>
    struct Type : public BaseT
    {
        Type(int length)
            : BaseT(length, "TEST FIELD PACKAGER", v)
        {}
    };
};
 
template<
      BaseT
    , ConstructorT
    >
class Foo
{
    typedef typename ConstructorT::template Type<BaseT> TestPackager;
 
    void run() {
        TestPackager(10);
    }
};
 
int main() {
    Foo<Bar, ThreeArgConstructor<bool, false> >().run();
    return 0;
}

A significant portion of your activities aren’t visible to your management, possibly because

• you have become the “product expert” which consults with others or
• your products’ production deployments require significant developer attention

Therefore,

Divide your whiteboard up into columns representing your work flow (e.g. “Backlog,” “Ready”, “Develop”, “QA”, “Done”) and obtain a large supply of Post-It(tm) notes. Use separate colors for the items assigned to you by your management and the items which come to you as the domain expert. If necessary, track enough information to build a cumulative flow diagram.

Discussion

There is something visceral about this approach that conveys understanding in a way which is not possible by explanation or discussion. It may be the case that your management knows what is happening but doesn’t quite “get it.”

This is a minimum useful subset of software kanban and an instance of “Information Radiator.”

A cumulative flow diagram can be created by simply counting the number of tickets in each bucket each morning and entering these into a spreadsheet in backwards order. You can read your average work-in-process (WIP) and cycle time from this diagram, making it an incredibly inexpensive way to measure the amount of effort spent on visible work versus other work.

Observed Instances

One

[to be filled in]

• Each release of your product has some unique aspect to it, for example an intended customer-specific configuration.
• Releases of your product are frequently made which fail in some, though not necessarily always the same, downstream step.
• Verifying correct operation of each build empirically is time consuming or costly.

Therefore

1. Write a program which is the simplest possible wrapper for your release of the artifact consumed downstream.
2. Each time a failure causes a flow interruption, modify the program to detect this problem with the artifact and fail loudly, refusing to produce the artifact.

Discussion

This pattern operates on the general principle that a shorter feedback loop for failures will have less negative effect on effort hours and throughput. This is the same principle embodied by test-driven development applied in a slightly different context. To build on the analogy, consider instituting a policy of adding the automated release check to your artifact-builder before correcting the downstream problem, and verifying the release check by attempting to release a new build without correcting the original problem.

This is a kind of test-driven development which is fed by downstream regressions. It is an attempt to introduce a positive rachet effect.

If you do not have some unique aspect to each release, you could simply add unit tests which fail the build if in-repository artifacts are somehow inconsistent or incorrect. Even though we don’t usually think to add unit tests to verify, say, properties of configuration files stored in the repository, these unit tests provide a tighter feedback loop than this pattern and should be preferred.

Observed Instances

One

We have a product where frequent, customer-specific releases are made to an independent QA department. Two files are delivered, an installer exe and an ini file. The ini file is read by the installer exe and causes the installer to install the correct components and install the proper configuration into the registry. The install is a lengthy process (twenty minutes) on machines which have a lot of software installed, discouraging developers from running the installer. Also, verifying correct operation requires setting up a protocol emulator which may not be available.

The ini file layout is very brittle. It contains many options which must be consistent with other options. There are also many options which must be specified for every component in the configuration. Yet other options can cause some other options to be ignored entirely. The install itself rarely fails, it leaves the machine configured in an unusable state instead.

A significant portion of my time was spent in the QA lab troubleshooting failed installs. When I first started maintaining this software, it seemed like one third of a typical day might be spent in the QA lab. Since QA is on an isolated network, I went through many thumb drives, not remembering which machine I had last left it in.

I wrote a wrapper for building the install after reading the ini file. This also had the benefit of allowing us to remove unused components from an install. For each failure found in QA, I made it fail loudly and abort before building an install if the problem was detected with the ini file.

I haven’t been in the QA lab in more than four months. I have kept the same thumb drive for perhaps a year, though I’m no longer sure what’s on it.

This has had an unexpected compounding positive effect: The most likely causes of an install failing are now either procedural or due to configuration changes made for testing in QA. Previously, the most likely cause of an install failing was that it had been packaged incorrectly. As a result of this shift, development is no longer the first on the scene of a failed install. Instead, the techs responsible for deploying the software to the customer handle most QA installation issues.

It might seem that a more fundamental problem in this occurrence is the brittleness of the configuration format, that it admits self-inconsistency. This is probably true; however, I could not determine a way to re-engineering the format piecemeal, and customer commitments (as well as time logged in the QA lab!) made such a large project unlikely on a maintenance-mode product. Also, the current installer has all sorts of difficult-to-understand logic with undocumented intent. This solution was truly cheap to get running.

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.

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.

• 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.

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.

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.

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");
    }

Here is a sample control chart.