patterncsharpMinor
Storing an integer counter in indexer
Viewed 0 times
storingintegerindexercounter
Problem
I've been implementing a custom internal server error page in ASP.Net MVC which will check if the current user is either an administrator or accessing the page from localhost, and if so, show them a whole bunch of details about the error to debug it with, otherwise just send them to a basic HTML error page.
So far, it works great, but one problem I had was that if there is an error in a partial view on the page, the system gets stuck in a loop trying to report the error.
To avoid this, I'm storing a temporary counter of how many times the current action has requested the error page in
Is there a better/prettier/shorter way of doing this?
So far, it works great, but one problem I had was that if there is an error in a partial view on the page, the system gets stuck in a loop trying to report the error.
To avoid this, I'm storing a temporary counter of how many times the current action has requested the error page in
TempData, but I find the amount of lines and style of the code to get, set and check this variable a bit verbose:using System.Web.Mvc;
namespace Test
{
public class ErrorController : Controller
{
[ActionName("500")]
public ActionResult InternalServerError(string aspxerrorpath = null)
{
int detectRedirectLoop = (TempData.Peek("redirectLoop") as int?) ?? 0;
TempData["redirectLoop"] = detectRedirectLoop + 1;
if((int) TempData.Peek("redirectLoop") <= 1)
{
// Check if user is admin or running locally and display error if so
}
return Redirect("/GeneralError.htm");
}
}
}Is there a better/prettier/shorter way of doing this?
Solution
I suggest the following:
public class ErrorController : Controller
{
private const string RedirectLoopCounterName = "RedirectLoopCounter";
private const int MaxRedirectLoopCount = 1;
private int RedirectLoopCounter
{
get { return ((int?)TempData.Peek(RedirectLoopCounterName)) ?? 0; }
set { TempData[RedirectLoopCounterName] = value; }
}
private int IncreaseRedirectLoopCounter()
{
return ++RedirectLoopCounter;
}
[ActionName("500")]
public ActionResult InternalServerError(string aspxerrorpath = null)
{
var isRedirectLoop = IncreaseRedirectLoopCounter() > MaxRedirectLoopCount;
if(isRedirectLoop)
{
return Redirect("/GeneralError.htm");
}
// Check if user is admin or running locally and display error if so
}
}- Create a constant for the counter's name as already mentioned by @BCdotWEB
- Create a property for getting and setting its value
- Create a method for actually increasing the counters value
- Additionaly you could replace the
1by a constant
- Finally you can replace the condition by a helper variable to document it better
- I would also invert the
if
Code Snippets
public class ErrorController : Controller
{
private const string RedirectLoopCounterName = "RedirectLoopCounter";
private const int MaxRedirectLoopCount = 1;
private int RedirectLoopCounter
{
get { return ((int?)TempData.Peek(RedirectLoopCounterName)) ?? 0; }
set { TempData[RedirectLoopCounterName] = value; }
}
private int IncreaseRedirectLoopCounter()
{
return ++RedirectLoopCounter;
}
[ActionName("500")]
public ActionResult InternalServerError(string aspxerrorpath = null)
{
var isRedirectLoop = IncreaseRedirectLoopCounter() > MaxRedirectLoopCount;
if(isRedirectLoop)
{
return Redirect("/GeneralError.htm");
}
// Check if user is admin or running locally and display error if so
}
}Context
StackExchange Code Review Q#102510, answer score: 7
Revisions (0)
No revisions yet.