0

So far I've learned that (usually) when you assert, you should also throw. At first I wasn't sure it was right, it just made sense1, and then this answer verified.

The problem I have is that it causes repeated-code (at least via contracts). Here's an example of a name field that can't be null or white-space:

 public String Name
 {
     get { return _name; }
     set
     {
         Contract.Requires(String.IsNullOrWhiteSpace(value) == false, "Names cannot be null or whitespace.");
         if (String.IsNullOrWhiteSpace(value))
             throw new ArgumentException("Names cannot be null or whitespace.");

         _name = value;
     }
 }

Clearly both the condition, and the error message are repeated. How should I handle it? In one property it's manageable but in a class with multiple fields it's littering.

Additional Information

I thought of using Contract.Requires<TException> (which would assert in debug and throw in release), but the condition (which is the code) would be the message of the exception, which would most likely be displayed to the user (a user would probably like to know why his program crashed, and I wouldn't want to show the actual code). Here's an example:

static void MyFunction(Boolean youCanSeeMyCode = false)
{
    Contract.Requires<Exception>(youCanSeeMyCode, "(This is the exception's message)");
}

...

try
{
    MyFunction();
}
catch (Exception ex)
{
    MessageBox.Show(ex.Message, "Error");
}

Error Message

Any suggestions? I'm at a loss.

1 It made sense because if you only use assert and you still have a bug somewhere that you're unaware of, the release build would carry on as if everything's okay and hell could break loose.

Community
  • 1
  • 1
MasterMastic
  • 20,711
  • 12
  • 68
  • 90

1 Answers1

2

The answer you've linked to is about Debug.Assert, which is turned off for release builds by default. I've never liked that idea - it's like saying that it's only worth wearing a seatbelt when you're learning to drive, but not when you're driving at speed.

I would encourage you to just use Contract.Requires (no separate explicit throw statement) and then make sure that you configure your project so that preconditions are still checked in release builds. Your concern about the message being displayed to the user isn't a concern which should be addressed here - it should be addressed in your general exception handling. If you don't want to show exception messages to the user, don't show them... but do log them. You really shouldn't try to make exceptions meaningful to end users... that's not their purpose.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thank you very much. I **loved** the seatbelt analogy, it's exactly what I think. I have just 2 questions. A) Why to enable `Contract.Requires` instead of using `Contract.Requires`? B) How can I display an error message to the user then? how would I identify each exception in-order to know what to "say"? – MasterMastic Mar 01 '13 at 15:23
  • @Ken: I meant `Requires` in general - not `Requires` vs `Requires`. All preconditions, basically. You shouldn't identify each exception - if an exception gets as far as the outer level, you should probably give a bland exception message saying that something is badly wrong, and provide a way of the user sending you the logs. It should be an *exceptional* occurrence for an exception to end up uncaught (or in this case for a precondition to fail) - the kind of thing you can't really anticipate. – Jon Skeet Mar 01 '13 at 15:25