Debug.Assert-s use the same error message. Should I promote it to a static variable?

Question

I love Asserts and code readability but not code duplication, and in several places I use a Debug.Assert which checks for the same condition like so:

Debug.Assert(kosherBaconList.SelectedIndex != -1, "An error message along the lines - you should not ever be able to click on edit button without selecting a kosher bacon first.");

This is in response to an actual bug, although the actual list does not contain kosher bacon. Anyhow, I can think of two approaches:

private static readonly mustSelectKosherBaconBeforeEditAssertMessage = 
    "An error message along the lines - you should not ever be able to " +
    "click on edit button without selecting a something first.";
...
Debug.Assert(
    kosherBaconList.SelectedIndex != -1, 
    mustSelectKosherBaconBeforeEditAssertMessage)

or:

if (kosherBaconList.SelectedIndex == -1)
{
    AssertMustSelectKosherBaconBeforeEdit();
}
...
[Conditional("DEBUG")]
private void AssertMustSelectKosherBaconBeforeEdit()
{
    // Compiler will optimize away this variable.
    string errorMessage = 
        "An error message along the lines - you should not ever be able to " +
        "click on edit button without selecting a something first.";

    Debug.Assert(false, errorMessage);
}

or is there a third way which sucks less than either one above? Please share. General helpful relevant tips are also welcome.

Answer 1

Keep it Simple, I would go for the static mustSelectKosherBaconBeforeEditAssertMessage = " ". And maybe even for inline strings if there were a few small differences.

Your AssertMustSelectKosherBaconBeforeEdit() looks too much like a premature/micro optimization.

Answer 2

Use the first (repetitive) form.

The reasons for this are:

• It's less work. Asserts are debug-only, so the repetition doesn't add any inefficiency to the release build. Don't waste time trying to make code "pretty" or "elegant" when it is already perfectly readable and functional.

• I'd argue it's actually shorter, more readable, and more easily maintained than your alternatives.

• When you hit an Assert in your debugger, it's very useful to have the full message in the code so you can see what the assert means. A lot of programmers use Assert(kosherBaconList.SelectedIndex != -1); with no explanation at all, which is fine if you wrote the code, but when you hit someone else's Assert you have no idea what it means. i.e. I know I called your method "wrong", but what the heck do I have to fix to call it "right"? The assert message should therefore indicate what the problem is, and what the person staring at the Assert() call needs to do to fix it. And you want it right there in front of you when your debugger stops in the middle of your test run.

• Sometimes you'll find it is useful to add additonal information relating to the context in which the assert fired, so you don't have to actually debug the code to work out which of your 3 "identical" asserts failed. I often use assert text like "BaconManager.Cook: You must select..." and "BaconManager.Wrap: You must select..." for this reason.

• If you include the text in the Assert call, it will not be compiled into your release build. But as soon as you make it a static variable, you will include the (unused) string in your release build, clogging it up with debugging junk.

A subroutine could be used if you want to check more than one condition to test the validity of your object, i.e.

[Conditional("DEBUG")] 
private void AssertValidity() 
{ 
    Debug.Assert(kosherBaconList.SelectedIndex != -1, "Nothing selected in kosher bacon list"); 
    Debug.Assert(kosherBaconList.COunt > 0, "kosher bacon list is empty!"); 
} 

Answer 3

One more suggestion: rather than static variable, make it:

private const string mustSelectKosherBaconBeforeEditAssertMessage = 
    "An error message along the lines - you should not ever be able to " +
    "click on edit button without selecting a something first.";

More on const vs static readonly: http://blogs.msdn.com/b/csharpfaq/archive/2004/12/03/274791.aspx

The difference is that the value of a static readonly field is set at run time, and can thus be modified by the containing class, whereas the value of a const field is set to a compile time constant.

Also here: http://msdn.microsoft.com/en-us/magazine/cc300489.aspx