7
\$\begingroup\$

I have an object that's generated from an incoming json string. The string is deserialized in a custom converter that returns an object. This object has a method called IsValidForDB that checks if the properties are valid. This method looks like this:

public class MyObject
{
   public bool IsValidForDB()
   {
      if (this.SomeProperty != SomeCondition)
      {
          return false;
      }

      if (this.SomeOtherProperty != SomeOtherCondition)
      {
          return false;
      }
      ....... several other conditions

      return true;
   }
} 

My question is this: is this pattern a good way of validating an object's properties?

\$\endgroup\$

migrated from stackoverflow.com May 2 '13 at 11:18

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ Are the conditions you check all 100% different from each other? If there is any way you can code it more generically you should. Long if/else if chains are ugly ugly. \$\endgroup\$ – Patashu May 1 '13 at 23:40
  • \$\begingroup\$ @Patashu: yes, all conditions are essentially different \$\endgroup\$ – frenchie May 1 '13 at 23:42
  • 2
    \$\begingroup\$ Have you looked at fluentvalidation.codeplex.com \$\endgroup\$ – kd7 May 2 '13 at 0:11
  • \$\begingroup\$ You may try to use data annotations attributes from System.ComponentModel.DataAnnotations Here is an example odetocode.com/Blogs/scott/archive/2011/06/29/… \$\endgroup\$ – user1614543 May 3 '13 at 8:41
1
\$\begingroup\$

You have two things to consider here:

  • the entity shouldn't know anything about any database or repository. I would suggest you rename your method to IsValid().
  • the entity will be manipulated or operated upon by other classes, i.e. view models or controllers, etc. It can be an anti-pattern (or just smelly) to include business rules in the entity when those business rules rightfully belong elsewhere

So, I would suggest that the validation within your object simply evaluates the property values in an immediate way, avoiding any business rules.

\$\endgroup\$
  • \$\begingroup\$ If entities are not business rules aware you run the risk of people setting in appropriate values if they manage the bypass the API in some way. Additionally when you have multiple access points to the entity the business logic would get repeated, not ideal in my opinion. \$\endgroup\$ – Basarat Ali May 2 '13 at 4:37
  • \$\begingroup\$ @BasaratAli There is a difference between "business objects" and "data entities". Of course you would put the business rules in a logical place - like behind some WCF services, so that they cannot be bypassed. You could argue the point forever, but nothing is absolutely perfect and there's always going to be a "What if...?". Business rules usually involve references to services and/or arbitrary application layers that data objects have no business knowing anything about. \$\endgroup\$ – slugster May 2 '13 at 6:37
  • \$\begingroup\$ yup ... those business rules shouldn't go in entities. Thanks for your explanation. \$\endgroup\$ – Basarat Ali May 2 '13 at 7:04
1
\$\begingroup\$

This answer is post preferred pick

This pattern is fine if exiting on the first invalid condition is what you fundamentally want. OF COURSE what if scenarios bring other considerations into the validation code design. As the code evolves refactor the validation structure as needed.

I agree with @slugster - change the name to IsValid() (or Validate(), whatever) because ...

"IsValid" vice "IsValidForDB" method name is a paradigm shift

A valid business object is one thing. Putting the data into a database is another, separate, thing. Do not put database features/limitations in your object's business rules.

The generic IsValid() means we are validating the object for problem domain business rules. Valid or not the database puts demands on us that are not problem domain validation issues per se. For example how do we store string.empty? As NULL, the blank character, or some other arbitrary character(s)?

So changing the method name acknowledges these separate concepts and therefore we codify these separate concepts literally. Code that must transliterate values to/from the database should be in some data layer of code independent of your business object.

\$\endgroup\$
0
\$\begingroup\$

If you need to check all of these conditions and they are all different then it would seems there's no getting around having to validate them in this sort of manner. Is validation during deserialization an option (and return an error or mark the deserialized object as faulty). Or checks on the db instead.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy