0

I am using a library, RapidXML, but my problem is more general. The library parses xml like item->first_node("CRAP")->first_node("CRAP") Now, if I put this in an if statement it will crash. If I put this: item->first_node("CRAP") it won't.

I am a beginner in C++ and I don't know much about exceptions but:

try
{
    if(item->first_node("CRAP")->first_node("CRAP"))
    {

    }
    cout << "OK";
} catch (...)
{
    cout << "CRASH";
}

The above crashes. How to check if my node exists without crashes (and without looping all the items one by one)?

Blazer
  • 235
  • 2
  • 9

3 Answers3

2

You simply need to take it one step at a time:

if (item != 0) // check if item is null
{
    rapidxml::xml_node<char>* node = item->first_node("CRAP"); // Try to grab first child node
    if (node != 0)
    {
        // okay got a valid node, grab next one
        rapidxml::xml_node<char>* next = node->first_node("CRAP");
        if (next != 0)
        {
           // Okay
        }
    }
}

When you try it in one step, i.e. item->first_node("CRAP")->first_node("CRAP"), you never check that the first call to first_node returned a null pointer (assuming item is a valid pointer also).

Jesse Good
  • 48,564
  • 14
  • 115
  • 165
2

Sounds like either item is NULL or item->first_node("CRAP") is returning NULL. Try this, see what output you get:

try
{
    node *n; // <-- use whatever type first_node() actually returns

    if (!item)
        cout << "item is NULL";
    else
    {
        n = item->first_node("CRAP");
        if (!n)
            cout << "first node is NULL";
        else
        {
            n = n->first_node("CRAP");
            if (!n)
                cout << "second node is NULL";
            else
                cout << "OK";
        }
    }
}
catch (...)
{
    cout << "CRASH";
}
Remy Lebeau
  • 505,946
  • 29
  • 409
  • 696
2

Always test whether an expression is NULL before using it as part of a longer expression. Never write things like

if(item->first_node("CRAP")->first_node("CRAP"))

if first_node("CRAP") can return NULL. Instead, write something like

if(item->first_node("CRAP") && item->first_node("CRAP")->first_node("CRAP"))

This works because the '&&' (logical and) operator uses lazy evaluation: it won't bother to evaluate its second operand if the first one evaluates to false.

Ernest Friedman-Hill
  • 79,064
  • 10
  • 147
  • 183
  • Heh. Hard to type good answers on an iPhone, don't know why I even try. – Ernest Friedman-Hill Dec 20 '13 at 03:44
  • @ErnestFriedman-Hill The backtick is available by holding down the quote key on the punctuation keyboard. – Potatoswatter Dec 20 '13 at 03:45
  • @RemyLebeau -- clarity is more important than performance, except when it's not. Oftentimes a single conditional expression is preferable to a couple of nested blocks, unless you're in a tight loop. – Ernest Friedman-Hill Dec 20 '13 at 03:46
  • 1
    It might be worth mentioning that the reason this works is because of [lazy evaluation](http://stackoverflow.com/questions/4613551/good-practice-in-c-lazy-evaluation) (for those who might not be aware of it). – JBentley Dec 20 '13 at 03:47
  • Yes but if I have 20 items (ITEMS1->ITEM2 etc), how to check it without making my code x100? – Blazer Dec 20 '13 at 03:50
  • @Blazer That's a great question, and the answer really is that an API that returns `NULL` to mean "not found" is difficult to use this way. A nicer API could return a special "null object" that returned "false" or other null object values from all its methods. You can read about this idea here: http://en.wikipedia.org/wiki/Null_Object_pattern . An API designed this way is much more convenient to use with chained method calls. – Ernest Friedman-Hill Dec 20 '13 at 03:55
  • @Blazer: Well, you can write a simple wrapper that observes Null Object Pattern as suggested. – Siyuan Ren Dec 20 '13 at 04:56