Another day, another “hit me hard” encounter with weird coding practices. The code below has been extracted from an ASP.NET MVC 5 project. It implements an action for a controller and the names have been modified. Only the main issue remains: some developers (I can no longer call them young or rookies or otherwise) create a model as a string. Of course when they check the string they do it case sensitive!

public ActionResult Action173( string email)
{
   if( String.IsNullOrEmpty( email ) )
   {
       return View();
   }
   
   var succeeded = BackendService.ExecuteAction173( email );
   return View( "{view name here}", succeeded ? "success":"failed" );
}

My first question when seeing the code was: how is the string used in the page? Is it actually displayed in the page?
No, the code in the view is something like:

var pageLayout = (String.IsNullOrEmpty(@Model) )? "invoice": @Model.Equals("success")? "success":"failed";
...
@if( pageLayout == "invoice")
{
...
}

@if( pageLayout == "success" )
{
...
}

@if( pageLayout == "failed" )
{
...
}

So why not define an enumeration in the controller sources (where R# quickly helps you move the definition to another file) and then switch on the passed enumeration? This way the compiler catches immediately a broken parameter.

Advertisements