I know that I am doing the validation wrong. Please convince me to stop :)

Question

First let me explain how I am currently processing validation, for example for an IPv4 address:

public struct IPv4Address { private string value; private IPv4Address(string value) { this.value = value; } private static IPv4Address CheckSyntax(string value) { // If everything fine... return new IPv4Address(value); // If something wrong with the syntax... throw new ApplicationException("message"); } public static implicit operator IPv4Address(string value) { return CheckSyntax(value); } public static implicit operator string(IPv4Address address) { return address.value; } } 

I have a bunch of structures like this.

They often have additional private members to handle things, but no methods are publicly disclosed.

Here is a usage example:

 IPv4Address a; IPv4Address b = "1.2.3.4"; a = b; b = "5.6.7.8"; string address = a; // c contains "1.2.3.4" IPv4Address c = address; // See if it valid try { IPv4Address d = "111.222.333.444"; } catch (ApplicationException e) { // Handle the exception... } 

I feel like something is really bothering there, so I'm thinking about switching to a static class using methods like IsIPv4Address, etc.

Now, I think it’s wrong with the above approach:

• New team members will have to wrap their heads around this

• This may interfere with third-party code integration.

• Exceptions are expensive

• I have never seen anything like it, and I am of a conservative nature :)

And then I like:

• Very close to having many specialized primitives since you have value types.

• In practice, they can often be used as primitive types, for example, this is not a problem by passing the above structure to a method that accepts a string.

• And, my love, you can pass these structures between objects and make sure they contain a syntactically valid value. It also avoids the need to always verify the correctness, which can be costly if done unnecessarily and even forgotten.

I cannot find the fatal flaw of the approach above (just a beginner here), what do you think?

Edit: since you can infer from the first line, this is just an example, I am not asking for a method of checking the IP address.

Answer 1

First of all, you should read messages about implicit casting, and when to use it (and why it is bad for use in your script), you can start here .
If you need validation methods, they should be public and not such strange constructions, in addition, having such methods, you can choose whether you want to throw exceptions (for example, .Parse() methods) or signal by returning some value that should be checked (e.g. .TryParse() methods).
In addition, having static methods to create valid objects does not mean that you cannot use a value type (struct) instead of a class if you really want to. Also, remember that structures have an implicit empty constructor that you cannot “hide”, so even your construct can be used as follows:

 IPv4Address a = new IPv4Address(); 

which will give you an invalid structure (the value will be null).

Answer 2

I also really like the concept of simulating primitive patterns within a framework, so I would follow this and IpV4Address.Parse("1.2.3.4") towards IpV4Address.Parse("1.2.3.4") (along with TryParse ), not implicit.

Answer 3

It seems like a lot of complexity, without benefits. Just because you can doesn’t mean what you should.

Instead of CheckSyntax (a string value) that returns IP (this method is poorly worded), I would just have something like

 bool IsIP(string) 

You can then put this in a utility class, or a base class, or a separate abstraction somewhere.

Answer 4

From the MSDN topic on implicit conversions

Predefined implicit conversions always succeed and never raise exceptions to throw. Properly designed by the user implicit conversions should demonstrate these characteristics also

Microsoft itself has not always adhered to this recommendation. The following use of the implicit System.Xml.Linq.XName operator throws an XmlException:

 XName xname = ":"; 

Perhaps the designers from System.Xml.Linq left him without documenting an exception System.Xml.Linq