Reputation: 46651
I have a map which displays some info. By default, labels on the map are turned off. There is an image button which has a turned-off image associated with it when you the labels are off and a turned-on image associated with it when you turn the labels on. I have the code working, but I wanted a better reason as to why it works this way. Here is a snippet of code.
If I declare a class-level boolean variable showLabels like:
private bool showLabels = false;
and then have the following code:
if(showLabels == false)
{
showLabels = true;
imgShowLabels.ImageUrl = "label-on.png";
}
else
{
showLabels = false;
imgShowLabels.ImageUrl = "label-off.png";
}
When I run it, the map comes up with the labels not shown by default. When I click on the Show Labels button, the variable showLabels becomes true and the image is changed to label-on.png, but when I click it again, the showLabels variable is reset to false, so nothing happens.
So what I did was change it from:
private bool showLabels = false;
to
private static bool showLabels = false;
and it is working this way.
Is this the correct way to handle this type of scenario?
In the class level, I put the property:
public bool ShowLabels
{
get { return (bool)ViewState["ShowLabels"]; }
set { ViewState["ShowLabels"] = value; }
}
In the if(!Page.IsPostBack), I am setting ShowLabels to false;
Then in my if statement, I am doing:
if(ShowLabels == false)
{
ShowLabels = true;
imgShowLabels.ImageUrl = "label-on.png";
}
else
{
ShowLabels = false;
imgShowLabels.ImageUrl = "label-off.png";
}
But ShowLabels is always false, I thought by setting the ViewState through the property ShowLabels, it would retain its value.
Upvotes: 1
Views: 1032
Reputation: 54894
No because this is a multi-threaded multi-user application, as all web applications typically are. You are essentially turning on the labels for every user on your site when it is enabled as true, if you set a variable to static.
Since you said this was ASP.NET you probably want to store this in the ViewState. You can do this by:
ViewState["ShowLabels"] = false;
And you can wrap this in a property.
public bool ShowLabels {
get { return (bool)ViewState["ShowLabels"]; }
set { ViewState["ShowLabels"] = value; }
}
Hope this helps. The alternative to ViewState is the SessionState, which you can use the same construct for.
public bool ShowLabels {
get { return (bool)Session["ShowLabels"]; }
set { Session["ShowLabels"] = value; }
}
Upvotes: 1
Reputation: 416149
Remember, each and every time you do a postback (for any reason, including just to process server events) you are working with a brand new instance of your page class. The old one is dead and gone- it was eligible for disposable as soon as the page was last rendered to the browser.
It "works" when you change it to static because static variables aren't associated with any particular instance. However, I suspect you'll be surprised when you start testing that in a wider environment. Since all users of the page for that site are in the same AppDomain, they'll all be setting the same showLabels variable.
To fix this, you need to store this variable somewhere that persists for just that user. Options include ViewState, Session, a database, or some other long-term storage mechanism. My preference leans toward the database (perhaps using the ASP.Net Personalization provider with sql server), because users will likely want you to remember this every time they come to your site.
How 'bout:
ShowLabels = !ShowLabels;
imgShowLabels.ImageUrl = string.Format("label-{0}.png", (ShowLabels)?"on":"off");
Upvotes: 8
Reputation: 25533
Joel hit the nail on the head, but I would add that what you should probably do is put the functionality on the client so to make sure your static variable isn't cleaned up unexpectedly.
I would do something like:
<a href="[YourPageHere.aspx]?showLabel=[togglethisvalue]>Show Labels</a>
For Webforms. Then you can process that as a query variable. This way you're gauranteed to have the right value. There are a ton of other ways to do this on the client side. I'm using the query variable approach for simplicity, but this might not work the more robust your pages become.
Upvotes: 2
Reputation: 46168
Not sure about the ASP, but as a style thing, having
if(showLabels == false)
isn't very readable. You should use
if(!showLabels)
instead
Upvotes: 1