CODEWITHSUNDEEP
c#class-designstructure
user10178
user10178

Reputation: 532

Validate constructor data

A sample class in "C# Class Desing Handbook" (pg 137) does not call the classes validation method for a specific field from inside the classes only constructor. So basically the sample class allows you to create an object with bad data and only throws an error for that data when you call the field's property which does validation on it then. So you now have a bad object and don't it know until after the fact.

I never understood why they don't just call the property from the constructor thus throwing an error immediately if bad data is found during initialization? I've emailed them to no avail...

I tend to use the following format by calling my properties from my constructors - is this proper structure to validate initialization data? ty

class Foo
{
    private string _emailAddress;

    public Foo(string emailAddress)
    {
        EmailAddress = emailAddress;
    }

    public string EmailAddress
    {
        get { return _emailAddress; }
        set
        {
            if (!ValidEmail(value))
                throw new ArgumentException
                    (string.Format
                    ("Email address {0} is in wrong format", 
                    value));

            _emailAddress = value;
        }
    }


    private static bool ValidEmail(string emailAddress)
    {
        return Regex.IsMatch
            (emailAddress, @"\b[A-Z0-9._%+-]+" +
                           @"@[A-Z0-9.-]+\.[A-Z]{2,4}\b",
                           RegexOptions.IgnoreCase);
    }
}

Upvotes: 3

Views: 2269

Answers (5)

Eric J.
Eric J.

Reputation: 150228

It makes no sense to me not to validate data in the constructor. As you point out, the object can end up in an invalid state. Given this design, you would not even realize that you had bad data when calling the getter.

For anything of moderate complexity or higher, I tend to use a Broken Rules approach rather than immediately throwing an Exception. In that approach, I define a BrokenRules object that contains information about the class and property that is invalid, and the reason that it is invalid. Then, in a common base class, I define a List to hold a list of everything "wrong" about the object. A property (again in the base class) IsValid indicates whether there are presently any broken rules.

The advantage of this is that there could potentially be several things wrong with the object state. If a user is being asked to correct the problems (i.e. this object is set from a UI), providing a list of all problems lets the user correct them in one go, rather than fixing one error just to be told there is another one. And another one. Etc.

Upvotes: 2

apiguy
apiguy

Reputation: 5362

the validation is happening when the Email address is set. This is where you want it because the email address could potentially be set again later.

If you also called validation in the constructor, you'd be making an extra, redundant validation call (once when constructed, another when the email address is set in the constructor).

Upvotes: 0

Noon Silk
Noon Silk

Reputation: 55172

Yes, if your general approach is:

Ensure that you can only get an instance of a valid object

then I love it.

Constructors should be used to create objects that are immediately valid, not to create just a 'container', for things to be put in.

Upvotes: 2

Kevlar
Kevlar

Reputation: 8924

I see nothing wrong with this approach. You can call methods on this within the constructor, and property setters/getters are just syntactic sugar for method calls.

Upvotes: 0

jrista
jrista

Reputation: 33010

Well, for one, you are likely to get the dreaded NullReferenceException, since you are not checking if emailAddress is null at any level. That particular check should be done in the constructor itself, and if emailAddress IS null, throw an ArgumentNullException. As for the rest, I don't see any particular problems with it as it is written in your sample. However, there are some issues that may arise if you make the property virtual, and derive children from this class. Execution order of field initialization, base and derived class consturctors then becomes an issue, and you have to be careful.

Upvotes: 2

Related Questions