Tom Gullen
Tom Gullen

Reputation: 61727

Basic boolean logic in C#

Which is better of the following?

this.isLoggedIn = (bool)HttpContext.Current.Session["li"] == true;

or

this.isLoggedIn = (bool)HttpContext.Current.Session["li"];

It needs be to true ONLY when the session is true. If the session is set to false will this evaluate to true in #2 as it exists? Or is it evaluating its value?

Upvotes: 0

Views: 1872

Answers (8)

x2.
x2.

Reputation: 9668

Both pieces of code are equal, so the better is the second (it's shorter).

Upvotes: -1

Arun
Arun

Reputation: 2523

I would use the second option with a variant:
this.isLoggedIn = (bool) (HttpContext.Current.Session["li"] ?? "false");

The ?? is null-coalescing operator - it gives a value of "false" to the expression on its lefthand side, in case it happens to be null.

Upvotes: 0

Jordão
Jordão

Reputation: 56477

Create a property for the desired value:

public bool IsLoggedIn {
  get { return (bool)HttpContext.Current.Session["li"]; }
}

You could even go one extra level, if the session is used a lot in the class:

public bool IsLoggedIn {
  get { return (bool)Session["li"]; }
}

private HttpSessionState Session {
  get { return HttpContext.Current.Session; }
}

Also, if you ever want to look at the session by itself, use a better key, like "IsLoggedIn", instead of "li".

It might be good to create a special class for these application-wide values:

public static class MyAppSession {

  const string IsLoggedInKey = "IsLoggedIn";

  public static bool IsLoggedIn {
    get { 
      return Session[IsLoggedInKey] != null && (bool)Session[IsLoggedInKey]; 
    }
    internal set { Session[IsLoggedInKey] = value; }
  }

  // ...

  private static HttpSessionState Session {
    get { return HttpContext.Current.Session; }
  }

}

Upvotes: 1

Oded
Oded

Reputation: 499002

The second one:

this.isLoggedIn = (bool)HttpContext.Current.Session["li"];

(bool)HttpContext.Current.Session["li"] is already a boolean (so will be either true or false), so no need for the extra comparison and return value of the boolean expression.

Either way, you need to check that the li session variable exists before trying to cast it, or your code will throw (I think a NullReferenceException).

Upvotes: 3

JK.
JK.

Reputation: 21808

You never need to write code that says:

bool x = (y == true);

Instead just use

bool x = y;

In your specific case you should use:

this.isLoggedIn = HttpContext.Current.Session["li"] != null 
    && (bool)HttpContext.Current.Session["li"];

This way you will not get an exception if Session["li"] has not been assigned yet. However you will get an exception if Session["li"] is not castable to bool.

Upvotes: 0

Fredrik Mörk
Fredrik Mörk

Reputation: 158309

Just put the expected value in the place of the expression, and it will become pretty clear:

First example:
Before: this.isLoggedIn = (bool)HttpContext.Current.Session["li"] == true;
After:  this.isLoggedIn = true == true;

Second example:
Before: this.isLoggedIn = (bool)HttpContext.Current.Session["li"];
After:  this.isLoggedIn = true;

Now, try the same for the false case:

First example:
Before: this.isLoggedIn = (bool)HttpContext.Current.Session["li"] == true;
After:  this.isLoggedIn = false == true;

Second example:
Before: this.isLoggedIn = (bool)HttpContext.Current.Session["li"];
After:  this.isLoggedIn = false;

As you can see, there will be no difference in the result between the two approaches. It all comes down to questions about coding style and readability, where I would guess that you would find a bias towards the shorter version.

Upvotes: 0

Øyvind Bråthen
Øyvind Bråthen

Reputation: 60694

The first and the second approach is equivalent, but the first one is to verbose for my taste. I like the second one much better.

Just as I like this

bool accepted = true;
if( accepted)
{
  ..
}

Better than

bool accepted = true;
if( accepted == true)
{
  ..
}

I feel it clearer that way if the variables are properly named.

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1500535

The latter is clearer, IMO. They're functionally equivalent though - in both cases, it will fetch the value of "li" from the session and attempt to cast it to bool, throwing an exception if the value isn't present.

Upvotes: 2

Related Questions