tags:

views:

563

answers:

4

This is my code:

public String[] readXML(String filename)  
{  
    XmlReader xmlReader = XmlReader.Create(@filename);  
    List<String> names = new List<string>();
    String[] keywords = null;
    while (xmlReader.Read())  
    {  
        //Keep reading  
        if (xmlReader.Name.Equals("Keyword") && (xmlReader.NodeType == XmlNodeType.Element))  
        {  
            // get attribute from the Xml element here  
            string keywords = xmlReader.GetAttribute("name");  
            names.Add(keywords);  
            String[] keywordsArray = names.ToArray();  
        }  
        else
        {
            MessageBox.show("An Error Occured");
        }
    }  
    return keywordsArray;
}

Would this do? Can someone test this?

+12  A: 

I think it is because your return statement is inside the if inside the loop. What happens if the loop never executes, or the if never is true?

FrustratedWithFormsDesigner
+3  A: 

You should have a default return for you while and if at the end of the method.

As the compiler says, you should have a return statement for all code paths. If the while never executes, not return, and the same for the if statement.

astander
+3  A: 

You have to put a return statement outside the loop. Otherwise if the name is never "Keyword", your code will never return!

NickAldwin
+4  A: 

Do you understand you own code? What do you want to return from this method? Currently your code means following:
Read XML file until "Keyword" element will not be found. After "Keyword" element will be found take value of it's "name" attribute and return it wrapped into one element array.
As others have mentioned there can be case when XML document will not have element. In that case you will exit your loop with no value returned. You should decide what to do in that case. You can return some special value like "null" or empty array. Or if you are sure that at least one "Keyword" element should exist in specified XML document then you can throw some Exception from the end of method to indicate that some pre-condition was violated.
I think you've tried to write method with different behavior. I believe you've meant following:
Read XML file and collect values of "name" attribute from all "Keyword" elements and return collected values as string array.
That will be more meaningful and in that case you will always have some result even when XML file will not contain any "Keyword" element. To achieve such behavior you should move list to array conversion and return statement to the end of method. You will have code like this:

public static String[] readXML(String filename)
{
    using(XmlReader xmlReader = XmlReader.Create(@filename))
    {
        List<String> names = new List<string>();
        while(xmlReader.Read())
        {
            //Keep reading  
            if(xmlReader.Name.Equals("Keyword") && (xmlReader.NodeType == XmlNodeType.Element))
            {
                // get attribute from the Xml element here  
                string keywords = xmlReader.GetAttribute("name");
                names.Add(keywords);
            }
        }
        String[] keywordsArray = names.ToArray();
        return keywordsArray;
    }
}

Please note that I've also wrapped usage of created XmlReader in "using" block to ensure that it will be disposed before we will leave the method.

Dmitriy Matveev