CODEWITHSUNDEEP
c#clr
MrPaulch
MrPaulch

Reputation: 1418

How to handle uninitialized local variables

After reading this Eric Lippert Article, I understand that the C# compiler doesn't like it if we leave local variables uninitialized.

As I encounter this 'problem' from time to time, I looked at some of my old code and was able to weed out most of the situation where actually don't need uninitialized (SomeClass obj = null) local variables.

But I came up with a situation where I don't know how to refactor the code.

public void DoSomething(string foo) {   

    SomeClass obj; // = null; 

    try {
        obj = SomeClass.CreateItem(target);
    } catch(CustomException ex) {
        // notify UI of error
    }

    if (obj != null) {
        // do something with `obj`
    }
}

SomeClass.CreateItem may fail due to external factors. If it does, I want to notify the user, if not I want to perform an Action.

The C# compiler doesn't want me to leave obj uninitialized, so I usually assign null to it.

This feels like a 'hack' now and my question is:

Is there a design flaw in the code above?

And if there is, how should I deal with references at compile time, when I can't determine if they are going to point to an existing object at run time?

Upvotes: 5

Views: 1117

Answers (3)

Eric Lippert
Eric Lippert

Reputation: 660138

I would refactor the code like this:

private SomeClass TryToCreateItem()
{
    try 
    {
        return SomeClass.CreateItem(target);
    } 
    catch(CustomException ex) 
    {
        // notify UI of error
    }
    return null;
}

public void DoSomething(string foo) 
{ 
    SomeClass obj = TryToCreateItem(); 
    if (obj != null) {
      // do something with `obj`
    }

"Extract method" is my favourite refactoring.

Upvotes: 10

gmiley
gmiley

Reputation: 6604

You could refactor it to encapsulate all your code within the try/catch related to that object and if you really do need to do something if it fails then you can use a bool to relate that to the rest of your code:

    public void DoSomething(string foo)
    {

        bool failedCreation = false;

        try
        {
            SomeClass obj = SomeClass.CreateItem(target);
        }
        catch (CustomException ex)
        {
            // notify UI of error
            failedCreation = true;
        }

        if (failedCreation)
        {
            // do other stuff.
        }
    }

But this doesn't look like what you have in mind. I would just encapsulate everything within the try/catch and be done with it.

Upvotes: 0

Servy
Servy

Reputation: 203834

The // do something withobj`` code should be inside the try block`.

What you're trying to do is run some code that may or may not succeed, and then run some other code only if the previous code succeeded. That's generally a very strong sign that the other code is a part of the same logical block that is dependent on there not being an exception. If there's an exception constructing this object you want this code to be skipped, which is exactly the behavior that you get by including it in the try block.

Upvotes: 2

Related Questions