Function Return paths

Question

I've got the following code and I really don't like the 'return string.Empty;' at the end but it's the only way I can keep Visual Studio from barking at me.

Can someone give me some advice on avoiding this? Or when or how to use 'using'

Also are there good resources on TDD? I can't figure out how to write tests for this.

private string GetWMIProperty(string property)
{
    SelectQuery selectQuery = new SelectQuery("Win32_OperatingSystem");
    using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(selectQuery))
    {
         string value = string.Empty;
         foreach (ManagementObject mo in searcher.Get())
         {
              return mo[property].ToString();
         }
    }
    return string.Empty;
}

Answer 1

Problem : you are returning from the foreach loop so that it returns only first item.

Solution 1: if you are sure that you have only one item that willbe returned from foreach loop then you can save that in a string variable and return at the end.

Try This:

private string GetWMIProperty(string property)
{
 string value = string.Empty;
 SelectQuery selectQuery = new SelectQuery("Win32_OperatingSystem");
 using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(selectQuery))
 {

   foreach (ManagementObject mo in searcher.Get())
   {
     value = mo[property].ToString();
   }
 }
return value;
}

Solution 2: if your foreach loop can iterate for more than 1 time then its better to save your items into some Collection and then return that Collection instead of string

Try This:

private List<string> GetWMIProperty(string property)
{
 List<string> value =new  List<string>();
 SelectQuery selectQuery = new SelectQuery("Win32_OperatingSystem");
 using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(selectQuery))
 {

   foreach (ManagementObject mo in searcher.Get())
   {
     value.Add(mo[property].ToString());
   }
 }
return value;
}

Answer 2

Every exit point of a procedure must be covered if the procedure has a return value.

If you can't find a property, perhaps you consider that an error and you should raise one:

throw new Exception("Cannot find property");

Answer 3

It really depends on how you want to handle the exceptional case where no property has been found.

If it's really not expected, then your use case is definitely a good candidate for an exception being thrown:

private string GetWMIProperty(string property)
{
    SelectQuery selectQuery = new SelectQuery("Win32_OperatingSystem");
    using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(selectQuery))
    {
        ...
    }

    thrown new ArgumentException("no WMI property found for specified property name", "property");
}

If it's expected in some cases and it's OK, then just return a default value as you already did (such as string.Empty).

Answer 4

It's a little subtle, but the problem is, when you put your return in a foreach loop you are effectively doing something like this:

if (myCollection.Count > 0) 
{
    return myCollection[0];
}
else
{
    // This path doesn't have a return value!!!
}

That's why VS is complaining. Since you are only returning the first value, perhaps it would be better to do:

 return searcher.Get().FirstOrDefault();

Instead of the foreach loop.

Answer 5

If you create a variable outside of the "using" block to hold the mo[property] values generated within the foreach loop, you will be able to use a single return statement at the end of the method block as shown below. It looks like you're expecting more than one string to be generated within the using statement, in which case you will want to make the "value" variable and the return type a list of string.

private List<string> GetWMIProperty(string property)
   {
       List<string> values = new List<string>();
       SelectQuery selectQuery = new SelectQuery("Win32_OperatingSystem");
       using (ManagementObjectSearcher searcher = new ManagementObjectSearcher(selectQuery))
       {
           foreach (ManagementObject mo in searcher.Get())
           {
               values.Add(mo[property].ToString());
           }
       }
       return values
   }