Object Validation

[Three5Eight]

Ok, I tried it again. I think it might still be wrong.

[TheCPUWizard]

Starting to look better. Your setters now to inherent validation.

But lets look closer...

       public string FirstName
        {
            get { return this.m_firstName; }
            set
            {
                PersonValidator pval = new PersonValidator();
                if (!pval.FirstNameValid(value))
                {
                    throw new ArgumentException();
                }
                m_firstName = value;
            }
        }

The code (neglecting the silver/gray text) is being DUPLICATED, and remember the mantra that CODE DUPLICATION IS EVIL.

You want to factor this entire part so that it exists ONLY ONCE, no matter how many fields "Person" has, and no matter how many different business entities you create.

So you create a class:

class ValidatedString
{
   public  string Value
        {
            get { return this.m_Value; }
            set
            {
                if (!m_Validator.Validate(value(value))
                {
                    throw new ArgumentException();
                }
                m_Value= value;
            }
        }
     public ValidatedString(Validator validator)
     {
          m_Validator = validator;
     }}

Now you NEVER have to type in or paste that construct again. You can simply use it:

class Parson
{
     public readonly ValidatedString FirstName;
     public readonly ValidatedString LastName;

     // Setup an Assign Validators in the Constructor
    public Parson()
    {
         FirstName = new ValidateString(new FirstNameValidator());
         LastName = new ValidateString(new LastNameValidator());
}

See how much smaller your class is??? NO Duplication, and Just 2 lines (as opposed to close to 20) for each field....

[Three5Eight]

I think it's getting closer then, but its still not working. I think I'm starting to understand what you are doing but I'm having trouble getting the syntax correct. I think I need a Validator interface or something. Because this wont compile. I'll try to fix it.

Sorry this is taking so long, but I look at your profile and remember you have been programming computers longer then I've been alive, so it comes easier to you.

David V. Corbin was an original founding member of Dynamic Concepts in 1984.

Dang. I wasnt even boren in 1984. No wonder the examples don't make sense to me.

[TheCPUWizard]

1) All your "validators" should derive from Validator. every one of them should have a single method called void Validate(string)

2) You are missing a new when creating the fields.

That should get you to compile.

ps: Yes, I have ben doing this for over 36 years [since sept 1972]....

[Three5Eight]

OK, it compiles now so I think I'm almost there. Except I know that PersonValidator.ValidateString won't work you can see why. I think this is what you talked about before in your first code example.

I know I need to pass some STRING_TYPE in there. I can add it in the param, "FIRST_NAME" but then that will break my code in ValidatedString.Value and I'll be having a bunch of duplicated code again if I do it the way I want.

I think you meant I can use generics to create my own string type which will hold the VALUE of the string and the TYPE (firstname, lastname, middlename) of the string.

I don't want people typing in my code though with data that can be corrupted:

MyStringType value<"Bob", "FIRST_NAME">;

So should I create an enum?

enum StringType
{
    FirstName,
    LastName,
    MiddleName,
    SomeOtherString
}

MyStringType value<"Bob", StringType.FirstName>

ValidateString(MyStringType value)
{
    switch (value.StringType)
{
        case StringType.FirstName:

            // Validate
         break;

         case StringType.LastName:
             // Validate
         break;
}
}

[Three5Eight]

What if I just added a m_valueType to the ValidatedString class. That might work instead. Then I would need to update my functions to take two arguments, and I would still need an Enum.

[TheCPUWizard]

You are overcomplicating it....

class Validator
{
    public virtual void Validate(string data) { }
}
class LengthValidator : Validator
{
   public LengthValidator(int min, int max) 
   {
        m_Min = min;
        m_Max = max;
   }

    public virtual void Validate(string data) 
      { 
          if (String.IsNullOrEmpty(data); } throw new Exception();
          if (data.Length < m_Min) throw new Exception();
          if (data.Length > m_Max) throw new Exception();
      }
}

        FirstName = new ValidateString(new LengthValidator(1, 10));
        LastName = new ValidateString(new LengthValidator(2, 15));

[Three5Eight]

ooook,

I guess I don't need all that extra consts in the validation class then because the sizes go when the field was created.

I'm pretty sure I can figure it out now. Then I will test it and see if it works. The few lines that are confusing me I put below. I think they were causing me the most trouble understanding this.

     public readonly ValidatedString FirstName;

You made this read only so when I test this I don't think I will be able to assign a first name at run time. Only in the constructor. This information might change from time to time so should I remove the readonly?

LastName = new ValidateString(new LengthValidator(2, 15));

This one I don't understand, I don't think. It looks like I'm making LastName into an instance of ValidateString... ohh **** nevermind. I got it, duh.

[TheCPUWizard]

1) You only create the field once (From the constructor), that is why ist MUST be readonly.

2) When you are changing the Values, you use the Value Propery:

   FirstName.Value="John";
   LastName.Value="Smith";
   Console.WriteLine(FirstName.Value);

[Three5Eight]

I'm stupid.

[TheCPUWizard]

I'm stupid.

If that from before my last post, or do you not understand my last post????

ps: I am going out for a few hours, it may be the monring (I am in NY USA) before I get back.

[Three5Eight]

No. I figured it out. I am stupid because it took all day to figure it out and it was so simple.

Thanks for all the help. I think I will be able to get it working now. If I get another problem I will post back.

[BigEd781]

You're not stupid, this happens all of the time when making software. The next time you try this you will already know how to do it, that's what learning is all about.

[Three5Eight]

So is it OK to leave unimplemented functions?

    public abstract class Validator
    {
        public virtual void ValidateString(string m_string)
        {

        }
    }

This works fine now, but if I name ValidateString abstract, add abstract ValidateDateTime to the validator class then I get an error in my LengthValidator ("Does not implement inherited ValidateDateTime"). But only DateTimeValidator should implement that. So I used virtural and left no implementation. Its an abstract class so I think this will be ok and it will allow derived classes to implement only the functions they want to use and ignore the rest.


Second, since I throw exceptions in LengthValidator will this code fail? If ValidatedString throws an exception will the value still be written into m_value?

        public string Value
        {
            get { return this.m_value; }
            set
            {
                m_validator.ValidateString(value);
                m_value = value;
            }
        }

[Mutant_Fruit]

One nice trick which I was introduced to a while ago is for when you check public methods to ensure the parameters passed in are non-null. What you do is something like this:

static class Check
{
    void DoCheck (object o, string name)
    {
        if (o == null)
            throw new ArgumentNullException (name);
    }

    void Path (object path)
    {
        DoCheck (path, "path");
    }

    void Stream (object stream)
    {
        DoCheck (stream, "stream");
    }
}


public void PublicMethod (string path, Stream stream)
{
    Check.Stream (stream);
    Check.Path (path);
}

This makes it easy to keep your null checks in sync with the names of the parameters. If you rename a parameter, you just call the corresponding Check method. While it isn't a huge saving in terms of LOC (1 line instead of 2), it is definitely more readable and can be easier to maintain.