Fun with ?: operator

First of all, take a look at the following code:

        private string _targetText;
        private int _maxLines;
        private int _maxSize;
 
        public int Lines
        {
            get
            {
                if (_targetText == null)
                    return 0;
                return _targetText.Split('\n').Length;
            }
        }
 
        public bool IsValid
        {
            get
            {
                return  _maxSize == 0 ?
                    true :
                    Size <= _maxSize
                    &&
                    _maxLines == 0 ?
                    true :
                    Lines <= _maxLines;
            }
        }

It’s fairly simple, the most important piece is IsValid property, that checks if _targetText meets certain length and number of lines limitations.

Now, let’s say that  max size is 5, max lines is 1 and target text is “Some incredibly long piece of text“. Million dollar question is: What would IsValid return for these parameters?

 

It would actually  return true, because there is a subtle bug in this code. It may not be apparent and and it’s a tricky beast because you have to know how to look at it to see what’s actually going on. The reason why it returns true, when all signs on earth and in heaven say it should return false is operators priority, and the way how ?: gets translated by compiler to some other code.

Logically thinking we would expect the code to examine if maxSize is 0 and if it is to set left hand flag to true, and if it’s not zero to set it to whether or not Size is less or equal max size, then to do similar thing with maxLines and Lines and set right hand flag, and then, if both flags are true to return true, and false otherwise. By thinking this way we assume that it will first run both ?: operators and then && the results, in other words, we assume that ?: operator has higher priority than && operator that turns not to be true.

That’s because people think of ?: operator like shorthand of if else, whereas mighty Reflector reveals its true nature to be different. When we compile code above and then open it in Reflector we’ll see code like this (line breaks and indents added to make it easier to read).

        public bool IsValid
        {
            get
            {
                return 
                    (
                        (this._maxSize == 0) 
                        || 
                        (
                            (
                                (this.Size <= this._maxSize) 
                                && 
                                (this._maxLines == 0)
                            ) 
                            || 
                            (this.Lines <= this._maxLines)
                        )
                    );
            }
        }

I suspect that this code looks slightly different than what you expected. No if else only logical ands and ors. And it’s the reason for that unexpected output. If you examine that code closely you’ll notice that no mater size and max size – if number of lines is not greater than maxLines it will return true. So how to fix that code? Either by surrounding ?: operators in brackets, or by moving them to other properties/methods like this:

        public bool IsValid
        {
            get
            {
                return HasValidSize
                    && HasValidLineCount;
            }
        }
 
        public bool HasValidLineCount
        {
            get
            {
                return _maxLines == 0 ?
                    true :
                    Lines <= _maxLines;
            }
        }
 
        public bool HasValidSize
        {
            get
            {
                return _maxSize == 0 ?
                    true :
                    Size <= _maxSize;
            }
        }

I hope that was informative.