Tuesday, September 6, 2011

How would you optimize this piece of C# code?

csharpI’ve been digging into some code lately when getting into an existing project, and came across these two lines:
if (Math.Max(thisString.Length, otherString.Length) > Math.Pow(2, 31))
throw new ArgumentException("String too long");

Looking at the code I see right away that this is ported from some other language over to C#. It’s quite easy to optimize it…. how would you do it?


  1. This comment has been removed by the author.

  2. First attempt, optimize by removing Math operations and use a logical or

    if(thisString.Length >= Int32.MaxValue || otherString.Length >= Int32.MaxValue)

    would by the eye be more efficient.. But, when thinking of it, who could possibly have a 2 gig text-string (and not use a String Builder) ?

    Besides, string.Length returns an int, so the value could never exceed the int.maxvalue . ;)

  3. It depends on the context.

    Initially, probably just remove the two lines.

    First, strings are 2 bytes per character, so the real limit is around 1 billion characters because of the 2GB object size limit in .NET. Well, that's assuming there isn't some magic with string interning that can push that limit higher but whatever, been covered on Stack Overflow.


    The more robust answer would be to check a more reasonable length, again depending on context. If this is say something that is operating on passwords, 64 characters would probably be excessive.

  4. Removing them would be the way to go, as any of the strings will never be longer than what .Net can store.

    I agree that if the check should be there it would be to check for a sensible length and not a boundary.

    The code is actually comparing two texts, and max length will never be more than a couple of K, so no need to check.