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
  • 50,901
  • 14
  • 124
  • 166
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
  • 555,201
  • 31
  • 458
  • 770
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
  • 80,601
  • 10
  • 150
  • 186
  • 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