3
\$\begingroup\$

I have an input group:

<div class="input-group">
    <div class="input-group-addon">
        @Html.DisplayNameFor(m => m.StorageId)
    </div>
    @if(ViewData.ModelState["StorageError"] !=null && ViewData.ModelState["StorageError"].Errors.Count>0)
    {
        <div class="a" title="@ViewData.ModelState["StorageError"].Errors[0].ErrorMessage" data-toggle="tooltip" data-placement="left" id="storage">
            @Html.DropDownList("storageId", null, new { @class = "form-control",@style="border-color:red" })
        </div>
    }
    else
    {
        <div class="a" title="@String.Empty" data-toggle="tooltip" data-placement="left" id="storage">
            @Html.DropDownList("storageId", null, new { @class = "form-control" })
        </div>
    }
</div>

As you can see, if there are no errors, the site should render div without a red border and tooltip.

Do you think it's efficient and good-looking enough to leave it as it is, or does it need to be rewritten?

\$\endgroup\$
4
\$\begingroup\$

Basically, 95% of your code is the same between the two sections, so I'd prefer to use just a single instance with a bool on the page.

@{ 
    var error = ViewModel.ModelState["StorateError"];
    var hasErrors = error != null && error.Errors.Any();
}

<div class="a" title="@( hasErrors ? error.Errors[0].ErrorMessage : string.Empty" )
    data-toggle="tooltip" data-placement="left" id="storage">
        @Html.DropDownList("storageId", null, new { @class="form-control",
            @style=" @( hasErrors ? "border-color:red", string.Empty ) })
</div>

You should also get into the habit of using .Any() instead of Count() > 0 as Any will stop iterating after it finds the first item that matches the filtering (usually not a big deal, but the collection could get rather large, and it's a minor change to coding that could give big performance enhancements).

The only other thing I might recommend would be to have a CSS class for border-color:red instead of inline styling.

\$\endgroup\$
  • 2
    \$\begingroup\$ After changing all @{...} in to @(...) all works and looks like a charm. Thanks! \$\endgroup\$ – szpic Jul 28 '14 at 13:30
  • 1
    \$\begingroup\$ Ah, thanks. Sorry for that. It was first thing this morning, and it's been a month or two since I wrote in Razor. I'll edit. \$\endgroup\$ – krillgar Jul 28 '14 at 13:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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