CodeSOD: Validate Freely

Validation highlights the evolution of a programmer as they gain experience. A novice programmer, when given a validation problem, will tend to treat the string like an array or use substrings and attempt to verify that the input is the correct format. A more experienced programmer is going to break out the regexes. A very experienced programmer is going to just find a library or built-in method that does it, because there are better ways to use your time.

Andrea provides a rare example of a developer on the cusp between regexes and built-in methods.

public static bool isvalidIP(this string address){ if (!Regex.IsMatch(address, @"\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b")) return false; IPAddress addr; return IPAddress.TryParse(address, out addr);}

The first thing to note is that, based on the message signature, we know that this is a C# extension method. The static and this keywords tell us that. This means that this method can actually be invoked as if it were a member function of strings: "10.0.0.1".isvalidIP(). This is very much a code smell, in this case- unless the vast majority of strings this application works with are supposed to be IP addresses, it makes more sense to have a separate validation method. But that's me being picky, I suppose.

The first check done here is a regex check. In this case, if the string doesn't contain a word boundary, followed by 1-3 digits, followed by a dot, 1-3 digits, a dot, 1-3 digits a dot, 1-3 digits and finally another word boundary, it can't possibly be a valid IPv4 address. Which I don't think this is going to produce any false negatives, but it certainly produces some false positives: 999.999.999.999 is not a valid IP address, but passes the regex.

But that's okay, because they filter out the false positives by calling IPAddress.TryParse. This is the built in method that will take a string, and attempt to turn it into an IP address object. If it succeeds, it returns true (and stores the result in the out parameter). Otherwise, it returns false and stores a null in the out. This step makes the regex unnecessary.

Addendum: As pointed out in the comments, there's a deeper WTF, quoting from the docs:

For example, if ipString is "1", this method returns true even though "1" (or 0.0.0.1) is not a valid IP address and you might expect this method to return false. Fixing this bug would break existing apps, so the current behavior will not be changed.

So, TRWTF is legacy support, in this case, and the regex isn't wrong, just not the most efficient approach to ensuring dots in the string.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.