Is there a reason I shouldn't be testing a set of variables for 0 by testing their product?

user_144665 0
Viewed 131

Is there a reason I shouldn't be testing a set of variables for 0 by testing their product?

Often in my coding across different languages I will test a set of variables to do something if they all consist of zeros.

For example (c#):

if( myString.Length * myInt * (myUrl.LastIndexOf(@"\")+1) == 0 )

Instead of:

if( myString.Length == 0 || myInt == 0 || myUrl.LastIndexOf(@"\") < 0)

Is there a reason I shouldn't be testing this way?

if-statement optimization c#
3 Answers

You shouldn't do that because it's not obvious what you're doing. Code should be clean, readable, and easily maintainable.

It's clever, but it's going to make the next person who looks at your code have to "decipher" what your intent was by doing it that way.

user_7817540

Here are a few reasons. All are important, and they're in no particular order.

  • Short circuiting. In your first example, all three things will be evaluated even if they don't need to be. In some cases, this can be a real problem: short circuiting is nice because you can do things like if (myObj != null && myObj.Enabled) without throwing exceptions
  • Correctness. Is myString.Length * myInt * myUrl.LastIndexOf(@"\") == 0 actually equivalent in all practical cases to if( myString.Length > 0 && myInt != 0 && myUrl.LastIndexOf(@"\") <= 0)? I'm not sure. I doubt it. I'm sure I could figure it out with some effort, but why should I have to in the first place? Which brings me to...
  • Clarity. Since the conventional way is to use separate statements and &&, anyone reading this code in the future will have a harder time understanding what it's doing. And don't make the excuse that, "I'll be the only one to read it", because in a few months or years, you'll probably have forgotten the thoughts and conventions you had when you wrote it, and be reading it just like anyone else.
user_7817920

There are practical cases in which that will not work.

Consider for example a string of length 65536 where the last character is a "\". 65536 * myInt * 65536 is always zero, because the product contains a factor 232 and int's are only 32 bits long. All the bits that would be set are not actually present in the result.

Depending on the value of myInt, that can also happen for shorter strings, but a string of 65k isn't that much of a stretch. (an URL of 65k may be a bit of a stretch, but whatever)

Note, by the way, that it's not so much the overflow that causes the problem, the problem is having several too-high powers of 2 as factor. For example, if all factors are odd (but high), the result will be odd as well (regardless of overflows), and therefore at least one bit will be set.

user_5550450
Related