Wednesday, January 7, 2009

Re-factoring

Prior to starting on the UI I have been doing some re-factoring. I have read that the important thing when re-factoring is that the code still works. My experience supports this. The ideal situation is that you re-factor in small steps and intersperse these steps with running unit tests that will ensure that you haven't broken anything. In the case of Cello as most of the work I have done is for functionality that allows Cello to mesh onto OSX, I have also written a lot of Unit Tests so I feel relatively secure about a modest amount of re-factoring.

The main things that I am doing re-factoring is:
  • Working through consistent naming.
  • Wrapping most the core foundation functions as methods.
Wrapping of core foundation functions as methods is something that I first came across in ACS (a part of MacApp). ACS aimed to present the what was originally the tool-box functionality as a set of C++ objects. In later versions following the introduction of CoreFoundation it did this, where possible, by defining the anonymous types. So in the case of CFData, a CFDataRef and CFMutableDataRef are defined as:
typedef const struct __CFData * CFDataRef;
typedef struct __CFData * CFMutableDataRef;
What ACS did was to then define the anonymous __CFData and add methods to it. So to add a method that would wrap CFDataGetLength it would did something like this:
struct __CFData
{
CFIndex GetLength() const;
};
and then defined an implementation for GetLength in terms of CFDataGetLength
inline CFIndex __CFData::GetLength() const
{
return CFDataGetLength(this);
}
When I started work on the Cello project I actively resisted this in favor of calling (typically) core foundation directly. What I found was that it made life a lot easier if I wrapped all the 'create' or 'copy' functions so that they returned boost::intrusive_ptr wrapped CF types. I did this as I used boost::intrusive_ptr to take care of all the ownership/ownercounting issues. When working on the Sound part of Cello I 'rediscovered' this ACS style wrapping, partly because it just made the code easier to read.

It means that instead of calling:
int length = CFDataGetLength(myData);
You write:
int length = myData->GetLength();
If you are dealing with boost::intrusive_ptr it eleminates a lot of 'get' calls so instead of
int length = CFDataGetLength(myData.get());
You write:
int length = myData->GetLength();
The other thing that comes into play is that I have had to wrap small amounts of Objective-C so that it can be used in C++. I have opted to keep Objective C out of the general C++ not just to properly isolate the complete madness of two different incompatible types exception handling but also to keep things simpler. It is generally natural to express these as C++ methods

No comments: