Mfcttrf

I have this function that takes a string which represents a html-color and returns the OLE value for it. If the string is null or empty or it can't be parsed, it uses a default value.

However I have to write that one line containing the default value twice and I don't see how I could remove this redundancy in the best possible way.

I know this isn't a big deal but I am curious to see what you guys might suggest for this :)

What do you think is the best possible way to remove this redundancy?

private int GetOleFromHTML(string stringRep)

 Color c;
 if (string.IsNullOrWhiteSpace(stringRep))
 
 c = Color.FromArgb(224, 224, 224);
 
 else
 
 try
 
 c = ColorTranslator.FromHtml(stringRep);
 
 catch
 
 c = Color.FromArgb(224, 224, 224);
 
 

 return ColorTranslator.ToOle(c);

Since a method has to be executed to get this color, you cannot declare the color as constant. A static readonly field is what comes closest to a const.

private static readonly Color DefaultColor = Color.FromArgb(224, 224, 224);

private int GetOleFromHTML(string stringRep)

 Color c;
 if (string.IsNullOrWhiteSpace(stringRep))
 
 c = DefaultColor
 
 else
 
 try
 
 c = ColorTranslator.FromHtml(stringRep);
 
 catch
 
 c = DefaultColor
 
 

 return ColorTranslator.ToOle(c);

Another approach would be to directly store the OLE color as int.

const int DefaultOleColor = 14737632; // R=224, G=224, B=224

private int GetOleFromHTML(string stringRep)

 if (string.IsNullOrWhiteSpace(stringRep))
 
 return DefaultOleColor;
 
 try
 
 return ColorTranslator.ToOle(ColorTranslator.FromHtml(stringRep));
 
 catch
 
 return DefaultOleColor;
 

Or calculated explicitly:

const int DefaultOleColor = 256 * (256 * 224 + 224) + 224;

Or with bit shift operations

const int DefaultOleColor = 224 << 16 | 224 << 8 | 224; // My preferred variant.

These expressions can be used to initialize the constant as they can be fully evaluated at compile time.

You can test these variants easily in the Immediate Window of Visual Studio. You must qualify the names with the namespaces for the first variant (System.Drawing.ColorTranslator.ToOle(System.Drawing.Color.FromArgb(224, 224, 224))).

Avoiding repetition is not the only reason for having constants. Constant declarations give a name to otherwise "magic" values. See Magic number (programming) (Wikipedia).

This variant avoids the repetition by restructuring the code, but still uses a constant.

const int DefaultOleColor = 224 << 16 | 224 << 8 | 224;

private int GetOleFromHTML(string stringRep)

 if (!String.IsNullOrWhiteSpace(stringRep))
 
 try
 
 return ColorTranslator.ToOle(ColorTranslator.FromHtml(stringRep));
 
 catch
 
 
 
 return DefaultOleColor;

Another option, though not without caveats, is to reverse the logic. Something like this should work:

private int GetOleFromHTML(string stringRep)

 Color c = Color.FromArgb(224, 224, 224);
 if (!string.IsNullOrWhiteSpace(stringRep))
 
 try
 
 c = ColorTranslator.FromHtml(stringRep);
 
 catch
 
 //Use default color value if the string is invalid.
 
 

 return ColorTranslator.ToOle(c);

You have difficulties getting rid of the repeated code because you're trying to force two different operations into one:

• parsing html


• converting the result into OLE color

Start with extracting the default value and make it a field:

static readonly Color DefaultColor = Color.FromArgb(224, 224, 224);

Then create the main API and give it a name that clearly communicates that it does. Now that the parsing has been extrated, you can as simple ternary operators here:

public Color ParseHtmlColorOrDefault(string value, Color defaultColor)
 
 return 
 !string.IsNullOrWhiteSpace(value) && TryParseHtmlColor(value, out var color)
 ? color
 : defaultColor;

The parsing with its try/catch as the TrySomething pattern:

private static bool TryParseHtmlColor(string value, out Color color)

 try
 
 color = ColorTranslator.FromHtml(value);
 return true;
 
 catch
 
 color = Color.Empty;
 return false;
 

Finally, create an extension for the final step of converting Color to OLE color:

public static int ToOle(this Color color) => ColorTranslator.ToOle(color);

Use it like that:

var oleColor = ParseHtmlColorOrDefault("#FFAABB", DefaultColor).ToOle();