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.

### Like this:

Like Loading...

*Related*

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