-

   rss_thedaily_wtf

 - e-mail

 

 -

 LiveInternet.ru:
: 06.04.2008
:
:
: 0

:


CodeSOD: And Now You Have Two Problems

, 13 2018 . 13:30 +

We all know the old saying: Some people, when confronted with a problem, think I know, Ill use regular expressions. Now they have two problems. The quote has a long and storied history, but Roger As co-worker decided to take it quite literally.

Specifically, they wanted to be able to build validation rules which could apply a regular expression to the input. Thus, they wrote the RegExpConstraint class:

public class RegExpConstraint
{
        private readonly Regex _pattern;

        private readonly string _unmatchedErrorMessage;
        protected string UnmatchedErrorMessage => _unmatchedErrorMessage;

        public RegExpConstraint(string pattern, string unmatchedErrorMessage)
        {
                _pattern = new Regex(pattern);
                _unmatchedErrorMessage = unmatchedErrorMessage;
        }

        /// 
        /// Check if the given value match the RegExp. Return the unmatched error message if it doesn't, null otherwise.
        /// 
        public virtual string CheckMatch(string value)
        {
                if (!_pattern.IsMatch(value))
                {
                        return _unmatchedErrorMessage;
                }
                return null;
        }
}

This neatly solved the problem of making sure that an input string matched a regex, if by neatly you mean, returns a string instead of a boolean value, but it introduced a new problem: what if you wanted to make certain that it absolutely didnt match a certain subset of characters. For example, if you wanted \:*<>|@ to be illegal characters, how could you do that with the RegExpConstraint? By writing a regex like this: [^\:*<>|@]? Dont be absurd. You need a new class.

public class RegExpExcludeConstraint : RegExpConstraint
{
        private Regex _antiRegex;
        public Regex AntiRegex => _antiRegex;

        public RegExpExcludeConstraint()
                : base(null, null)
        {
        }

        /// 
        /// Constructor
        /// 
        /// Regex expression to validate
        /// Regex expression to invalidate
        /// Error message in case of invalidation
        public RegExpExcludeConstraint(string pattern, string antiPattern, string unmatchedErrorMessage)
                : base(pattern, unmatchedErrorMessage)
        {
                _antiRegex = new Regex(antiPattern);
        }

        /// 
        /// Check if the constraint match
        /// 
        public override string CheckMatch(string value)
        {
                var baseMatch = base.CheckMatch(value);
                if (baseMatch != null || _antiRegex.IsMatch(value))
                {
                        return UnmatchedErrorMessage;
                }
                return null;
        }
}

Not only does this programmer not fully understand regular expressions, they also havent fully mastered inheritance. Or maybe they know that this code is bad, as they named one of their parameters antiPattern. The RegExpExcludeConstraint accepts two regexes, requires that the first one matches, and the second one doesnt, helpfully continuing the pattern of returning null when theres nothing wrong with the input.

Perhaps the old saying is wrong. I dont see two problems. I see one problem: the person who wrote this code.

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!

https://thedailywtf.com/articles/and-now-you-have-two-problems

:  

: [1] []
 

:
: 

: ( )

:

  URL