Implementing IXmlWriter Part 1: The Basics

This is part 1 of my Implementing IXmlWriter post series.

After writing my blog post Don’t Form XML Using String Concatenation, I realized that writing a C++ System.Xml.XmlWriter workalike involves some interesting challenges. Therefore, I’ve decided to write a series of blog posts about building a streaming C++ XML generator, a.k.a. IXmlWriter, step-by-step. For this series, I will follow the practice of test-driven development and write a test case followed by an implementation which passes the test case. Future posts’ test cases will be constructed to illustrate bugs in or new features desired from the previous post’s implementation. The test cases will be constructed with the goal of having IXmlWriter be as similar to System.Xml.XmlWriter as possible.

This is the first post in that series. Today’s test case is:

StringXmlWriter xmlWriter;

xmlWriter.WriteStartElement("root");
  xmlWriter.WriteStartElement("element");
    xmlWriter.WriteString("Contents");
  xmlWriter.WriteEndElement();
xmlWriter.WriteEndElement();

std::string strXML = xmlWriter.GetXmlString();
// strXML should be <root><element>Contents</element></root>

Looking carefully at this test case, I see the following requirements for StringXmlWriter:

  1. It needs to support 4 functions: WriteStartElement(), WriteEndElement(), WriteString(), and GetXmlString().
  2. It must provide the ability to retrieve the XML as a std::string. For simplicity, I will simply keep the generated XML as a std::string member variable and return it from GetXmlString().
  3. It needs to keep track of what XML elements have been opened so that it can properly produce the end tag when WriteEndElement() is called. It is sensible for StringXmlWriter to contain this logic and not the caller (in other words, WriteEndElement() shouldn’t take the name of the element to close) because it doesn’t make sense for WriteEndElement() to close any other element other than the most recently opened one. The natural data structure to store this information is a stack, so I will use a std::stack.

    Remember, I am only concerned with passing this test case. To fix any bugs or deficiencies I must write a new test case first, and then fix the implementation accordingly. Keeping this in mind, here is the overly simple implementation which I came up with to pass this test case:

    class StringXmlWriter
    {
    private:
        std::stack<std::string> m_openedElements;
        std::string m_xmlStr;
    
    public:
        void WriteStartElement(const std::string& localName)
        {
            m_openedElements.push(localName);
            m_xmlStr += ‘<’;
            m_xmlStr += localName;
            m_xmlStr += ‘>’;
        }
    
        void WriteEndElement()
        {
            std::string lastOpenedElement = m_openedElements.top();
            m_xmlStr += “</”;
            m_xmlStr += lastOpenedElement;
            m_xmlStr += ‘>’;
            m_openedElements.pop();
        }
    
        void WriteString(const std::string& value)
        {
            m_xmlStr += value;
        }
    
        std::string GetXmlString() const
        {
            return m_xmlStr;
        }
    };
    
    Advertisements

Don’t Form XML Using String Concatenation

It seems very common for developers to create XML using string concatenation, as in:

std::string CreateXML
    (
    const std::string& strValue
    )
{
    std::string strXML("<tag>");
    strXML += strValue;
    strXML += "</tag>";
    return strXML;
}

As any experienced XML developer knows, this code has a bug: strValue must be escaped (& must be converted to &amp;, < must be converted to &lt;, etc.) or the XML that is generated will not be well-formed and will not be able to be parsed by an XML parser. One could write a simple function to handle this escaping, but there are so many other issues that can potentially creep up with XML generation—string encoding, illegal element and attribute names, collapsing empty elements, making sure all opened elements are closed, the performance of string concatenation, etc.—that I recommend putting all this logic into a single class. In fact, I suggest modelling the class after .NET’s streaming XML generator, System.Xml.XmlWriter.

Typically, this class (let’s call it IXmlWriter) will be instantiated high up on the call stack and passed as a parameter to functions which generate XML, as in:

void CreateXML
    (
    IXmlWriter& w,
    const std::string& strValue
    )
{
    w.WriteStartElement(std::string("tag"));
    w.WriteString(strValue);
    w.WriteEndElement();

    // Or w.WriteElementString(std::string("tag"), strValue);
}

If you still require the XML as a string, you can write an implementation of IXmlWriter to write to a resizable string buffer (such as ostringstream) and then get the buffer’s contents as a string once all XML writing has finished.

Writing this code is a bit more work but it will pay off in the end.

Update 2005-09-21 12:20 PM: Removed code fragment of what an ostringstream-driven IXmlWriter might look like in anticipation of future blog posts.

Be Careful With Doubles And C++ Streams

I ran across a piece of code recently that was using ostrstream to convert a double to a string. The code looked something like:

std::string DoubleToString(double d) {
    std::ostrstream ostr;
    ostr << d << std::ends;
    std::string str(ostr.str());
    ostr.freeze(false);
    return str;
}

This function was used to convert doubles to strings for insertion into an XML document, which were eventually parsed in an XSLT by the XPath number() function. Most of the time it worked fine, but for really large numbers the number() function failed and return NaN. Why?

The answer is that ostrstream defaulted to printing large doubles in scientific notation. This means that the statement std::cout << DoubleToString(10000000000000000); would print out 1e+016, a string that cannot be not parsed by number(). To fix this, you must tell ostrstream to output the number in fixed notation, as follows:

std::string DoubleToString(double d) {
    std::ostrstream ostr;
    ostr.setf(std::ios::fixed);
    ostr << d << std::ends;
    std::string str(ostr.str());
    ostr.freeze(false);
    return str;
}

Use RAII

This is covered by any halfway-decent C++ book, but I believe it deserves reiteration: Use the RAII idiom. I don’t think I could explain RAII any better than HackCraft does in The RAII Programming Idiom.

Let me demonstrate how to use RAII with a semi-contrived example. Here’s the pre-RAII code:

HMODULE hm = LoadLibrary(_T("user32.dll"));
if (hm != NULL) {
    FARPROC proc = GetProcAddress(hm, "MessageBoxW");
    if (proc != NULL) {
        typedef int (WINAPI *FnMessageBoxW)(HWND, LPCWSTR, LPCWSTR, UINT);

        FnMessageBoxW fnMessageBoxW = (FnMessageBoxW) proc;
        fnMessageBoxW(NULL, L"Hello World!", L"Hello World", MB_OK);
    }
    FreeLibrary(hm);
}

In this case, the resource wrapped is HMODULE, the resource acquisition function is LoadLibrary(), and the resource release function is FreeLibrary(). Beware of resources which have multiple resource release functions, such as Win32’s HANDLE with FindClose() and CloseHandle(); for these cases you will typically have to write multiple RAII classes. I will call the wrapper class HModule. Here’s how its use will look:

HModule hm(LoadLibrary(_T(”user32.dll”)));
if (hm != NULL) {
    FARPROC proc = GetProcAddress(hm, “MessageBoxW”);
    if (proc != NULL) {
        typedef int (WINAPI *FnMessageBoxW)(HWND, LPCWSTR, LPCWSTR, UINT);

        FnMessageBoxW fnMessageBoxW = (FnMessageBoxW) proc;
        fnMessageBoxW(NULL, L”Hello World!”, L”Hello World”, MB_OK);
    }
}

// FreeLibrary() will happen automatically when hm
// goes out of scope

Things to note:

  1. I replaced the assignment statement HMODULE hm = LoadLibrary(_T("user32.dll")); with the construction statement HModule hm(LoadLibrary(_T("user32.dll")));. On newer compilers this may not be necessary.
  2. HModule needs to handle the case where LoadLibrary() fails—it must not call FreeLibrary() on an invalid HMODULE. Be careful of types such as HANDLE as it uses both NULL and INVALID_HANDLE_VALUE to denote invalid values.
  3. HModule should be transparently convertible to a HMODULE. This implies either inheriting from HMODULE or, preferably, providing a conversion operator to HMODULE. (Some resource wrapping classes, like STL’s std::basic_string, prefer a method rather than a conversion operator (c_str() in this case), probably to minimize unexpected, implicit conversions. The choice is up to you.)
  4. If a user calls FreeLibrary(hm); that will result in a double-free, a potentially dangerous bug. I don’t think there’s a whole lot that can be done about this besides saying “Don’t do that!”
  5. We need to worry assignment and the copy constructor: the default implementations will lead to double-free bugs. A few solutions are: disabling the operators, but this makes the class less useful than it could be; turning assignment/copy construction into transfer of ownership like STL’s std::auto_ptr, but this causes some unexpected semantics; or using reference counting like Boost’s shared_ptr, but this adds often-unneeded complexity and, by itself, introduces the problem of circular references. For simplicity, I will simply choose to disable assignment and the copy constructor.

Given these observations, here’s my version of HModule:

class HModule
{
private:
    HMODULE m_hm;

public:
    explicit HModule(HMODULE hm) : m_hm(hm) {}
    ~HModule() { if (m_hm) { FreeLibrary(m_hm); }}
    operator HMODULE() const { return m_hm; }

private:
    // Disable copy construction and assignment
    HModule(const HModule& hm);
    HModule& operator=(const HModule& hm);
};

Obviously, there’s a lot more that can be done to HModule, such as supporting release of ownership, reassignment, etc. However, by using RAII classes like HModule exclusively, you will immediately reap the gains of less thought about resource management, fewer bugs, and you will be one step closer towards exception safety.

BTW, if you don’t want to go to the trouble of writing RAII classes, there’s also ScopeGuard.

Beware File Open/Save Dialogs: They May Change The Current Directory

On some Windows operating systems (primarily Windows 95, 98, and ME), GetOpenFileName() and GetSaveFileName() (and wrappers of these functions such as MFC’s CFileDialog) will permanently change the process’s current working directory unless the OFN_NOCHANGEDIR option is specified. As you can imagine, this can easily break your application if you ever rely on the current working directory being set to a particular value (such as if you open files using relative paths).

Of course, it is best to eliminate any such current working directory assumptions from your application completely.

Be Careful With Bitfields

Consider the following piece of code (adapted from a real-world bug):

BOOL IsDirectory(LPCTSTR strPathName)
{
    DWORD dwAtts = GetFileAttributes(strPathName);
    if (dwAtts == INVALID_FILE_ATTRIBUTES)
        return FALSE;
    return (dwAtts == FILE_ATTRIBUTE_DIRECTORY);
}

This function will work most of the time, but every now and again it will run across a directory which it will claim isn’t one. Why?

The problem is caused because dwAtts is a bitfield (see the GetFileAttributes() documentation). Simple equality comparison isn’t appropriate, as if the directory has any other attributes (such as compressed, hidden, offline, etc.) the comparison will fail.

To test if a bit is set in a bitfield, use code of the form (dwAtts & FILE_ATTRIBUTE_DIRECTORY) == FILE_ATTRIBUTE_DIRECTORY or (dwAtts & FILE_ATTRIBUTE_DIRECTORY) != 0.