Bad comparisons, part three

Did you notice how last time my length comparison on strings was unnecessarily verbose? I could have written it like this:

static int ByLength(string x, string y)
  if (x == null && y == null) return 0;
  if (x == null) return -1; 
  if (y == null) return 1; 
  return CompareInts(x.Length, y.Length); 

static int CompareInts(int x, int y) { 
  // positive if x is larger
  // negative if y is larger
  //  zero if equal   
  return x - y; 

static Comparison<T> ThenBy<T>(this Comparison<T> firstBy, Comparison<T> thenBy) 
  return (x,y)=> 
    int result = firstBy(x, y); 
    return result != 0 ? result : thenBy(x, y); 

Much nicer! My string length comparison method is greatly simplified, I can compose comparisons easily with the ThenBy extension method, and I can reuse CompareInts as a helper function in other comparisons I might want to write.

What’s the defect now?







This one should be easy after last time. The lengths of strings are always positive integers and in practice they never go above a few hundred million; the CLR does not allow you to allocate truly enormous multi-gigabyte strings. But though CompareInts is safe for inputs which are string lengths, it is not safe in general. In particular, for the inputs Int32.MinValue and Int32.MaxValue, the difference is 1. Clearly the smallest possible integer is smaller than the largest possible integer, but this method gives the opposite result! CompareInts should read:

static int CompareInts(int x, int y) 
  if (x > y) return 1; 
  if (x < y) return -1;
  return 0; 

The moral of the story here is that a comparison function that doesn’t compare something is probably wrong. Subtraction is not comparison.

Next time on FAIC: One more bad comparison.

1 thought on “Bad comparisons, part three

  1. Pingback: Bad comparisons, part two | Fabulous adventures in coding

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s