Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a problem. This code works great, but i think it can be reduced.

@if (Session["Success"] != null)
{
    <text>
        <div class="alert alert-success">@Session["Success"]</div>
    </text>
    Session.Remove("Success");
}
else if (Session["Error"] != null)
{
    <text>
        <div class="alert alert-danger">@Session["Error"]</div>
    </text>
    Session.Remove("Error");

}
share|improve this question
 
What should happen when Session contains both "Success" and "Error"? Should that behave exactly the same as your code (i.e. print the success case)? Or is that situation impossible? –  svick Dec 15 '13 at 15:05
 
What exactly is this code supposed to do? The title is extremely generic and should be replaced. –  Jamal Dec 15 '13 at 16:00
 
This code just show error or success result of any action. Exactly it's alert. –  soundwave1337 Dec 15 '13 at 16:06
add comment

1 Answer

up vote 5 down vote accepted

It's a bad idea to modify the session state from inside a view or a partial view. Instead, I would create an alert view model

public class AlertViewModel
{
    public AlertType AlertType { get; set; }
    public string Message { get; set; }
}

public enum AlertType
{
    None,
    Success,
    Error
}

and use that from inside the view/partial view, for example:

@if (Model.AlertType != AlertType.None)
{
    string alertClass = Model.AlertType.ToString().ToLowerInvariant();

    <text>
        <div class="alert alert-@alertClass">@Model.Message</div>
    </text>
}

This way, you avoid modifying the session where someone else may not expect it, and you have a cleaner view.

About populating the view: I do not know why you are using the session, but if the value is only needed for one request, you should consider using TempData instead. In any way, just populate the view model in the controller and remove the session value if you need to.

share|improve this answer
 
Thank you! It will be very useful. –  soundwave1337 Dec 16 '13 at 14:47
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.