CODEWITHSUNDEEP
c#asp.net-mvc
Scott Rippey
Scott Rippey

Reputation: 15810

Do I need to initialize a variable, even if I know it will be assigned before used?

In an ASP.NET MVC app, I have code that boils down to the following:

public ActionResult Test() {
    string query;
    try {
        query = GenerateQueryString();
    }
    catch (Exception ex) {
        ModelState.AddModelError("error", ex.Message);
    }

    ... do additional validation ...

    if (ModelState.IsValid) {
        return RedirectToAction("Success?" + query);
    }

    return View(); // Show validation messages
}

The above code has a compile error ... query might not be initialized.

However, the logic in the code will clearly initialize query before using it.

What is the best way to resolve this issue?

Upvotes: 0

Views: 189

Answers (9)

Renatas M.
Renatas M.

Reputation: 11820

The thing here is that local variables must be initialized before read - they won't be initialized by default and this prevents from bugs programmers often makes. Some time ago I was looking for the answer to this simple situation:

bool a = true, b;
if (a == true) //here everything is OK, because a is initialized
   b = true;
if(b == false) //here you get an error, I thought b == false by default
   return;

I found on stackoverflow deep explanations, why it works so(couldn't find at the moment which I was reading). But you can read Absence of evidence is not evidence of absence interesting article. Maybe it will explain what I tried to say :)

Resume:

In your case you need to initialize query or set variable in catch block

Upvotes: 1

Paul Turner
Paul Turner

Reputation: 39685

The C# compiler is looking at your code and seeing that the value of the variable is initialized in a try block and is then used later on. It can see that a logical branch of execution exists where an exception is thrown, is caught, and then the subsequent code is executed with an uninitialized value.

The simplest way to stifle the error is to assign a default value to the field. null is sufficient. As you're adding the error to the MVC model state and are checking that value later on before accessing query, you shouldn't be in a situation when the default value will matter.

Upvotes: 2

Michael Stum
Michael Stum

Reputation: 181124

Just because YOU know that it will be initialized (it may not be by the way in your code), the COMPILER can't know that - it's called the Halting problem.

So the compiler errs on the side of caution, something .net does a lot (e.g., no fallthrough in switch..case).

Upvotes: 1

jensk
jensk

Reputation: 41

The Code is NOT clearly initialized. When the GenerateQueryString() Method throws an Exception, no value will be set. You should set the String to null and check for null before you use it, als you don't break in the catch-block.

Upvotes: 1

SWeko
SWeko

Reputation: 30942

If there is exception in the call to GenerateQueryString the value of query will be undefined.

Try

string query = string.Empty;

to have a definite assignment to the variable.
Alternatively, if an exception should abort the execution, this will work too:

string query;
try {
    query = GenerateQueryString();
}
catch (Exception ex) {
    ModelState.AddModelError("error", ex.Message);
    return View();
}

Upvotes: 1

Huske
Huske

Reputation: 9296

It could just be that your GenerateQueryString() throws an exception. Personally, I prefer to initialize my variables, just to be on the safe side. For this reason you might just initialize to query to:

string query = String.Empty;

It doesn't hurt to be on the safe side.

Upvotes: 0

Mark Bertenshaw
Mark Bertenshaw

Reputation: 5689

The problem is that query is initialised in the try block, but query is declared in the block above. You must initialise it at the top level block.

Upvotes: 0

Fischermaen
Fischermaen

Reputation: 12468

Just change the declaration of query in: string query = string.Empty;.

Upvotes: 0

Polity
Polity

Reputation: 15150

You are wrong, What if GenerateQueryString throws an exception? What will be the value of Query?

You might want to do a query = "Error"; or something in your catch block. This because appareantly you want the application to run if an exception in GenerateQueryString is thrown.

EDIT

I would recommend against presetting query with a default value. This because the meaning is different. A default value is different than a certain value at a certain moment. Doing so will also prevent the compiler from warning you when you create a new code path but forget to setup a value for query

Upvotes: 1

Related Questions