My suggestions to a client on improving their legacy code.
Initially:
- Move all source code from VSS into TFS.
- Ensure all code needed to compile is within the workspace.
- Ensure all dependencies are correct so compilation of any one module will automatically compile needed dependencies for that module.
- Add UnitTest++ (or CppUnit – whatever floats your boat) to project.
- Add Log4Cxx to project – more man-hours have gone into writing this than your own system.
As you develop new code or extend older code:
- Move the logical projects into folders. Create another physical project alongside this project. Usually you will call this Test or Tests.
- For each module you are editing in add a c++ source file in the test project. Format for this will be Test.cpp.
- Write the test for the part you are changing, this should be as concise as possible. See Feathers “Working Effectively with Legacy Code
” and Kerivsky “Refactoring to Patterns
” books for tips.
And eventually:
- Use Msbuild for automated building of source. This will build all code, every time, and warn of errors and can even bundle things into installers, etc.
- Turn on treat warnings as errors and fix.
- Get a static analysis tool and slowly fix the errors (only about 20 are relevant – the rest are pedantic).
- Make everyone use “Convert tabs to spaces” and set to 4. (HICPP)
- All if and else statements must be enclosed in braces, even if a single line. (HICPP)
- Apply SOLID principles to code (Single responsibility, Open/Closed, Liskov Substitution, Interface, Dependency Inversion).
- Evangelise to others in team.
Others:
- You should be able to get ~1,500 unit tests per second.
- Some things you can’t test (UI, threading) so try to separate code from logic at test that (DoSomethingUI and DoSomething, and read The Humble Dialog Box).
- It will seem at first you will be writing a lot of extra code with tests and refactoring, but stick with it.
Did I miss anything?
…but they definitely were not K-I-S-S-I-N-G. In fact they weren’t even holding hands.
As a test at work for a new project I was asked to look at using Maven, which is currently being used to great effect by the Java programmers, with C++. A new project has arisen that requires proof that there is 100% code coverage, has various code metrics and need to be presented at the end of it.
As usual the first thing I did was canvas the ‘Net for other peoples experiences. There is no point in reinventing the wheel if you can help it. Unfortunately peoples experiences weren’t good. On the plus side they weren’t recent.
So I set off looking at the Mojo Native Plugin. One of the first things that concerned me was the lack of direct Unit Testing support. I had noticed that several other language plugins had direct support for unit testing. However I can work around that. The other major concern was running the code metric tools that were being evaluated concurrently.
Eventually, after nearly whole frustrating week, I gave up with the Mojo tool. It was simply unworkable. Didn’t help that I am not use to paying the angle bracket tax. I then shifted my focus to the Jade Native Plugins. This is actually a fork of the Maven plugin but has moved along and supports unit testing and static analysers.
This on the face of it seems like a good thing but was again unworkable, but this is probably me more than them – I am not used to editing the pom.xml and getting the relevant info from provided examples is not easy especially when the underlying code seemingly doesn’t work with the examples.
Feeling increasingly like a failure, I turned to Hudson which several posts had pointed to. This was a breeze in comparison. A massive relief as I had a build up and running quickly and a couple of days later after overcoming a few niggardly problems I had a build that had unit tests and code coverage.
I presented my findings and guess what we picked? Option C: Maven2 execing batch files. Did I mention the PM is/was a Java programmer?
April 8th, 2010 :: graham.reeds
Categories:
C++ ::
Life ::
WorkI recently had a problem at work where an application failed if the user tried to use UNC paths in a application that was ran as a service.
This service was written in C# that called into C DLL. Not only that, but the DLL was written in VC6.
So after confirming I could recreate the bug I removed the service and installed the debug version. I tested again and it was still there. I then added System.Diagnostics.Debugger.Break to the code and stepped in and through until I got to the calls to the DLL. The call stepped straight over. Hmm.
- Debug build of the DLL: Check.
- PDB file: Check.
- Step into unmanaged code: Checked.
- Debugging option: Mixed.
So what next. Maybe it wasn’t picking up the debug DLL. So I went around and deleted every other version of the DLL. Didn’t fix it and broke most of the support apps. Maybe it was a weird VS2K5/VC6 interaction. So I rebuilt the library in Visual Studio but it still failed.
I could see the problematic code, but I couldn’t step in to see what the parameters were when it was there. So I created a small console app and transplanted in the problem code and called it. Worked as expected. Which brought up the idea that something was changing the parameters – but why, where and again why?! So the simple console app was extended to call the dll with every single call from the service in it with the same parameters, finally ending where the problem happened.
However it didn’t happen where I expected it to – the breakpoint I placed was never hit. I thought that it wasn’t debugging properly still but it broke successfully if I placed a breakpoint at the start of the code. But if the code wasn’t being reached where it called CreateDirectory how comes there is a directory in the root of C:\ with the name of unc machine?
It turns out that it called a function earlier in the code than expected that uses mkdir – and the code that uses CreateDirectory never gets used due to the parameter tests that are done.
I looked at this code and thought about fixing it but there was over a hundred lines of strcat, strcpy, strtok and moving pointers around. I decided that it was horribly broken and rewrite was in order. So I looked on the net because this is a good case of reinventing the wheel. Most sites, including my beloved StackOverflow, suggest using Boost.FileSystem which in my case is a non-starter. So I had a think about it and this is what I came up with after one false start:
BOOL CreateDirectories(char* path)
{
BOOL status = TRUE;
char* p;
char temp_path[256];
strcpy(temp_path, path);
do {
p = strrchr(temp_path, '\\');
*p = '\0';
} while (!SetCurrentDirectory(temp_path));
do {
p = strchr(temp_path, '\0');
*p = '\\';
status = CreateDirectory(temp_path, 0);// do not check because this will fail on 'too\\many_slashes'
} while (strlen(temp_path) < strlen(path));
return status;
}
Now you can get away with removing the temp_path but I copied the original string to be safe, and I should of used MAX_PATH instead of hard-coding 256 but I think the code is quite elegant (though the WordPress formatting doesn’t really do it justice).
There’s a warning there – not to compare against the status in the while clause as having two slashes instead of one will prematurely cause the function to exit. Ignore the error that the function returns (ERROR_ALREADY_EXISTS) and the following time through it will continue to create the directories.
NOTE: I found this in my drafts folder, from a long, long time ago, it’s a little dated but I will let it stand as some of the points are still valid.
At work I’ve inherited several legacy projects, one large with several smaller ones that support it. Making changes is pretty dangerous – I’ve been bitten a few times by a seemingly innocuous change that has only come to light during testing. The way I use the application as the developer and the way a service engineer uses the same application is different.
So I started writing unit-tests (UnitTest++ retrofitted to work with VC6) to test my assumptions and make sure that my changes don’t break things. A major problem is testing private functions. Some can be fixed by promoting private to protected, inheriting the class and then adding testing hooks in the class. Like:
// Example.h
class Example
{
bool ThisNeedsTesting();
};
// in TestExample.cpp
#include "Example.h"
class TestExample : public Example
{
public:
bool TestThisNeedsTesting() { return ThisNeedsTesting(); }
};
TEST( TestThisNeedsTesting )
{
TestExample tester;
CHECK( tester.TestThisNeedsTesting, true );
}
Others get refactored out the classes they reside in and get put into their own functor classes – the current project has validation that is needed in several subclasses. These were first on my list to be refactored – they became functors and as a bonus made it easier to test – just a call to their public () operator. Others were functions that performed several duties. These too were split into separate functions and then split into functors as they weren’t directly related to the class they were in – they just played a supporting role.
Then I started on the larger objects. These became strategies. These strategies each implemented states via enums. These are massive classes and are a testing and maintenance nightmare – and one of these is what needed changing. According to Gamma, et al, states have a main state with empty functions. Specific states are derived from these states like so:
// I've left out the public declarations to save space
class State
{
virtual void StateOne() { };
virtual void StateTwo() { };
};
class StateBored : public State
{
virtual void StateOne() { /* do stuff */ }
};
class StateHappy : public State
{
virtual void StateTwo() { /* do happy stuff */ }
};
This really aids maintenance, extending and unit-testing as well, but it is screaming out to be functors with just the () operator being overridden in each. Like this:
// I've left out the public declarations to save space
class State
{
virtual void operator() () { };
};
class StateBored : public State
{
virtual void operator() () { /* do boring stuff */ }
};
class StateHappy : public State
{
virtual void operator() () { /* do happy stuff */ }
};
I am also sure that I will be finding more functors as I continue. Which really reduces most coding to functions – like traditional C programming. Does anyone else find them reducing nearly everything to functors?