tags:

views:

189

answers:

4

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.

+1  A: 

Not sure about the ASP, but as a style thing, having

if(showLabels == false)

isn't very readable. You should use

if(!showLabels)

instead

thecoop
Or even better: reverse the order and just say _if (showLabels)_
Joel Coehoorn
thanks for the suggestions.
Xaisoft
I agree that if you need to test for both true and false in an 'if..else' the positive should go first ( as Joel ) suggested. But testing for false only using ' == false ' looks a lot more readable to me than '!'.
kervin
@kervin, I actually agree with that. You can easily miss the !
Xaisoft
+8  A: 

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");
Joel Coehoorn
So that explains why the showLabels variable is reset back to false when the page posts back. How do static variables respond when you do a postback? It appears to retain its value.
Xaisoft
So you are saying that if it is static, user A and user B will both be using showLabels and if user A has the labels off, how would that affect user B?
Xaisoft
Is it then better to use ViewState in this case?
Xaisoft
If userA changes showLabels, userB would see userA's preference. I would _not_ use ViewState for this, because users will likely to expect you to remember the preference for later visits as well. A database seems like a better place to put it.
Joel Coehoorn
I see what you are saying, what if a database is not feasible in this situation, I guess ViewState is really the only other option, correct?
Xaisoft
You should put this where ever you are keeping all of your stored user settings/data.
Joel Coehoorn
if a user is logged in, they are usually stored in the asp.net membership tables, but what if it does not require a user to login?
Xaisoft
ASP.Net personalization supports anonymous use, but assuming that's off then you just don't know who they are, and the Session is the best you can do.
Joel Coehoorn
Is session better than ViewState? I heard ViewState is is per page, but Session can span multiple pages?
Xaisoft
Yes: Session can span multiple pages. The downside is that by default it keeps everything in ram, which can hurt server performance. But one int/bool here and there shouldn't hurt much.
Joel Coehoorn
I am trying the ViewState first, but I am running into issues, I will update my code above.
Xaisoft
Question updated with ViewState code.
Xaisoft
@Xaisoft have a look at this: http://msdn.microsoft.com/en-us/library/2y3fs9xs.aspx. I think your best bet for long term persistence of these option is to use the ASP.NET Profile. This supports both anonymous and authenticated users.
RichardOD
If you don't like ViewState you can always use the Session in the same manor. See my example below.
Nick Berardi
+1  A: 

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; }
}
Nick Berardi
I'm a little confused about ViewState? What is the advantage of using ViewSate over a static variable in this case?
Xaisoft
ViewState is per-user, static is not. The downside here is that ViewState is also per-page view. I suspect users will want you to just remember this preference.
Joel Coehoorn
Is there a way to do this with a database if a user is not logged in.
Xaisoft
Viewstate basically handles the automatic persistence of variables by using HTML hidden variables. I'd recommend a good book on ASP.Net to understand it. Don't use Viewstate if you need any performance in your application at all. Store/Retrieve the variable in HTML yourself.
kervin
@kervin: there's nothing wrong with ViewState performance if you use it appropriately.
Joel Coehoorn
Dido Joel, ViewStates don't have problems, but people cramming megabytes worth of data in them is the problem that slows down webpages.
Nick Berardi
+2  A: 

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.

Joseph
thansk for the suggestion.
Xaisoft