From df0d556fcc2a50e1c7901e400cac23e2a08280f2 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 20 Jan 2015 17:45:28 -0800 Subject: [PATCH] Follow-up on 12565daf88603501521bf0315617be443fa5f705 PR comment - remove `explicitValue` variables from two `DefaultHtmlGenerator` methods This follows up a [code review comment](https://github.com/aspnet/Mvc/pull/1834/files#r23120381) related to `DefaultHtmlGenerator.GenerateCheckBox()`. Same issue appears in `DefaultHtmlGenerator.GenerateRadioButton()`. @pranavkm - > Also a different variable name? There's a parameter isExplicitValue that > follows which always has a false value. Reusing this name seems messy. My response to that comment should not be repeated. @pranavkm was correct that the only issue here is naming. `explicitValue` was too easily confused with the `isExplicitValue` argument used a few lines later. They had separate purposes: `explicitValue` related to the "checked" attribute while `isExplicitValue` relates to the "value" attribute. --- .../Rendering/Html/DefaultHtmlGenerator.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs index 5f3c565b9a..82b577eb9e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/DefaultHtmlGenerator.cs @@ -110,10 +110,9 @@ namespace Microsoft.AspNet.Mvc.Rendering } var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); - var explicitValue = isChecked.HasValue; - if (explicitValue && htmlAttributeDictionary != null) + if (isChecked.HasValue && htmlAttributeDictionary != null) { - // Explicit value must override dictionary + // Explicit isChecked value must override "checked" in dictionary. htmlAttributeDictionary.Remove("checked"); } @@ -124,7 +123,7 @@ namespace Microsoft.AspNet.Mvc.Rendering metadata, name, value: "true", - useViewData: (metadata == null && !explicitValue), + useViewData: (metadata == null && !isChecked.HasValue), isChecked: isChecked ?? false, setId: true, isExplicitValue: false, @@ -325,10 +324,9 @@ namespace Microsoft.AspNet.Mvc.Rendering string.Equals(model.ToString(), valueString, StringComparison.OrdinalIgnoreCase); } - var explicitValue = isChecked.HasValue; - if (explicitValue && htmlAttributeDictionary != null) + if (isChecked.HasValue && htmlAttributeDictionary != null) { - // Explicit value must override dictionary + // Explicit isChecked value must override "checked" in dictionary. htmlAttributeDictionary.Remove("checked"); }