CODEWITHSUNDEEP
c#
12shadow12
12shadow12

Reputation: 317

Forward catch from method

I'm trying to grab a ComponentId of a network device, it all works but now I am trying to grab a catch from the GetKey() method. I'm just not quite sure how to forward it (or if this is even the correct way). Should I even be using try/catch in methods?

static string GetKey()
{
    try
    {
        using (RegistryKey Adapter = Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\Class\{4d36e972-e325-11ce-bfc1-08002be10318}", true))
        {
            foreach (string Keyname in Adapter.GetSubKeyNames())
            {
                RegistryKey Key = Adapter.OpenSubKey(Keyname);
                if (Key.GetValue("DriverDesc").ToString() != "somename")
                { 
                     return Key.GetValue("ComponentId").ToString();
                }
                return null;
             }
             return null;
         }
     }
     catch (Exception ex)
     {
         return ex.Message;
     }
}


static void Main(string[] args)
{
    if (GetKey() == null)
    {
        Console.WriteLine("null");
    }
    else if () // if catch throws exception?
    {}
    else
    {
        Console.WriteLine(GetKey());
    }
    Console.ReadKey();
}

Upvotes: 0

Views: 405

Answers (4)

Janne Matikainen
Janne Matikainen

Reputation: 5121

You could write your GetKey in a bit more usable fashion with making it a TryGetKey instead just GetKey and handle exceptions. Like so

static bool TryGetKey(out string result)
{
    result = null;

    try
    {
        using (RegistryKey Adapter = Registry.LocalMachine.OpenSubKey(@"SYSTEM\CurrentControlSet\Control\Class\{4d36e972-e325-11ce-bfc1-08002be10318}", true))
        {
            foreach (string Keyname in Adapter.GetSubKeyNames())
            {
                RegistryKey Key = Adapter.OpenSubKey(Keyname);

                if (Key.GetValue("DriverDesc").ToString() != "somename")
                {
                    result = Key.GetValue("ComponentId").ToString();
                    return true;
                }
            }
        }
    }
    catch (Exception ex)
    {
        Trace.WriteLine(ex.Message);
    }

    return false;
}

And in your calling code you should be checking what the result of that function was in order to do something with the key or handle the failure.

var key = string.Empty;
if (TryGetKey(out key))
{
    // Got key can do something.
}
else
{
    // Something went wrong, what should the application do?
}

Also generally wrapping the entire code block within try-catch is considered bad practice.

Upvotes: 0

Ilya Chernomordik
Ilya Chernomordik

Reputation: 30235

This looks like a very bad idea to me to return the exception message in a method that returns a string (which is some key).

You need to understand: what is the reason for the try/catch? What are you trying to achieve there? If you just want to log the exception, it's better to do that in some top-level framework error handling than here.

If you want to hide some exceptions (which is generally a bad idea, but there are exceptions), then you can just return null there to indicate nothing has been found. Moreover it's quite unlikely that the exception happens after you have the value of the key you can return (or even impossible since you just return it).

It is also not 100% clear what exactly you want to forward. If it's the exception itself, then you can do this:

try
{
}
catch (Exception exception)
{
    throw;
}

Now this will properly rethrow the exception with original stack trace. While this code is useless on it's own, you can add some handling before throw if you need.

Upvotes: 2

Misza
Misza

Reputation: 665

Returning an exception message when the code clearly expects some key value is a very bad idea.

The typical way to do it is to rethrow the exception as your application-specific type, with the original exception as inner. So in your catch block in the method you do:

 catch (Exception ex)
 {
     throw new MyApplicationException(ex);
 }

Where MyApplicationException is your own exception type that you then handle in your main program using

catch (MyApplicationException ex)
{
    ////Handle it!
}

Upvotes: 0

BugFinder
BugFinder

Reputation: 17858

Try, thats what the try, catch, finally is for 

Try
{
    String result = GetKey();
    if (result == null)
    {
        Console.WriteLine("null");
    }
    else
    {
        Console.WriteLine(result);
    }
}
catch
{
    Console.WriteLine("Reading key failed");
}
    Console.ReadKey();

Note also I changed your code from repeatedly calling GetKey - which was unnecessary

Upvotes: 0

Related Questions