Reputation: 61727
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
Reputation: 9668
Both pieces of code are equal, so the better is the second (it's shorter).
Upvotes: -1
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
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
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
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
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
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
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