Troy Loberger
Troy Loberger

Reputation: 347

Validating information in a constructor

How bad of practice is this? I am currently being asked by my professor to do this which is against everything I have been told. Can anybody give me examples why you should not validate this way? (Using regular expressions in the get / set methods in a asp web page)

More Information:

Here is the code of what he wants us to do: In the Property:

public String FName
{
    get
    {
        return _fName;
    }
    set
    {
        if (validateName(value.ToString()))
            _fName = value;
    }
}

The method im calling:

public static bool validateName(String name)
    {
        bool isGood = true;

        Regex regex = new Regex("^[A-Z]");
        if (!regex.IsMatch(name))
            isGood = false;

        return isGood;
    }

Upvotes: 0

Views: 217

Answers (5)

Heather
Heather

Reputation: 2652

I agree with your instructor.

In general, you should validate a value in any place it is possible to set it prior to "accepting" it. The general rule is that whatever method that attempts to set the value should receive immediate feedback when it attempts to set it.

For your example, I would place the validator inside of the setter of your FName public property, and if your constructor also accepts a FName value, then simply call the FName setter within your constructor to fully encapsulate the behavior of the property, be it validation behavior or any other business rules that the property implements:

public class User
{
    public User(string firstName, string lastName)
    {
        FirstName = firstName;
        LastName = lastName;
    }

    private string _firstName;
    public string FirstName
    {
        get { return _firstName; }
        set
        {
            if (!IsValid(value))
                // throw / handle appropriately
            else
                _firstName = value;
        }
    }
}

Also: stay away from abbreviations! Do not use FName; use FirstName.

Upvotes: 1

RobH
RobH

Reputation: 3612

In reply to your update, I'd find that really annoying, for example:

var a = new yourObject();
a.FirstName = 123;

What my code doesn't know is that I've failed validation so I haven't changed the first name property at all!

Edit:

Your can also simplify your validation method:

public static bool validateName(String name)
{
    Regex regex = new Regex("^[A-Z]");
    return regex.IsMatch(name)
}

Upvotes: 1

gmn
gmn

Reputation: 4319

It depends what you mean by validation, guard clauses are quite common practice in constructors e.g.

if(param1 == null) 
     throw new ArgumentNullException("param1");

It helps make sure that your object is in a consistent state for use later on (preventing you having to check at the time of use).

You can also use guard clauses on properties (what your case seems to be) and methods too, to ensure your object is always in a consistent state.

Upvotes: 1

S. Ravi Kiran
S. Ravi Kiran

Reputation: 4303

Purpose of a constructor is to assign values to the members of a type. By convention, validation is not responsibility of constructor.
Validation of any information is dependent on business of the application that you are building. If you are creating a modular application where every component is meant for a specific purpose, it is better to create a separate class or a set of classes (depending on size of the application) to perform all business validations. Such validations have to be invoked depending upon the validation rules imposed on a piece of data.

Upvotes: 0

Tigran
Tigran

Reputation: 62248

In general it's not good, as validating as is, presumes also a failure.

So the questions are:

  • How do you intend to handle faults during constructor code execution. ?

  • What if you get an exception in constructor? What the state of the object remains after that ?

That's why it's a bad practice in general. The good path to follow is to:

  1. Construct object
  2. Run validation

But these are guides, and you're free to brake them based on your convenience. So in deffence of your professor, should say, that he asked this:

  1. Or to bring you to some thoughts
  2. Or to teach you something

So follow his path and try to understand why he asked to write the code in that way.

Upvotes: 1

Related Questions