tags:

views:

87

answers:

3

I have a method in C# that finds a node with name node_name in node list arg, and returns the value of found node (assuming there's only one node with such name). If no such nodes are found, it should return an empty string.

public string get_nodes_value(XmlNodeList arg, string node_name)
{
    foreach (XmlNode arg_node in arg)
    {
        if (!arg_node.HasChildNodes)
        {
            if (String.Compare(arg_node.ParentNode.Name, node_name) == 0)
            {
                return arg_node.Value;
            }
        }
        else
        {
            get_nodes_value(arg_node.ChildNodes, node_name);
        }
    }
    return "";
}

The code above always returns an empty string. What did I miss here?

+3  A: 

Well, you're ignoring the return value of the recursive call in the else block. Did you mean to return from there in some cases? My guess is you want something like this (fixing a few convention oddities at the same time):

public string GetNodeValue(XmlNodeList list, string name)
{
    foreach (XmlNode node in list)
    {
        if (!node.HasChildNodes)
        {
            if (node.ParentNode.Name == name)
            {
                return arg_node.Value;
            }
        }
        else
        {
            // Only return if we've found something within this node's child list
            string childValue = GetNodeValue(node.ChildNodes, name);
            if (childValue != "")
            {
                return childValue;
            }
        }
    }
    return "";
}
Jon Skeet
I think you have a bug when the node you search for is a leaf. You will always return an empty string.
Petar Minchev
@Petar: I'm assuming that the idea is to search for *text* nodes within *elements* of the given name. I'm just going on what the original code does though.
Jon Skeet
THanks for fast reply :)That was what I missed.
Nikita Barsukov
+1  A: 

Whichever recursive invocation finds your node will return it, but unless it's the top-level that value just gets ignored. You probably meant to do something like:

    else
    {
        string value = get_nodes_value(arg_node.ChildNodes, node_name);
        if (value != "")
            return value;
    }
tzaman
A: 

The easiest way to find that out would be to step through the code while setting up "watches" on arg_node.ParentNode.Name and node_name and then you'll see which branches it'll end up in and you can find out why it didn't go where you thought it would go.

ho1