Should I cache a comparer in a static field?

Question

I have added the following property to my ApplicationUser class, which is supposed to return current user on top of other results.

public static IComparer<string> IdComparer
{
  get
  {
    return Comparer<string>.Create((x, y) =>
        {
          var curUser = HttpContext.Current.User;
          if (curUser != null)
          {
            var curId = curUser.Identity.GetUserId();
            if (x == curId)
              return -1;
            else if (y == curId)
              return 1;
          }
          return string.Compare(x, y);
        });
  }
}

Anyway, does generating a comparer cost more than storing it? Should I add a static field and return a singleton for this property?

I'm thinking about returning the same comparer:

private static object sync = new object();
private static IComparer<string> _IdComparer;
public static IComparer<string> IdComparer
{
  get
  {
    if (_IdComparer == null)
      lock (sync)
        if (_IdComparer == null)
          _IdComparer = Comparer<string>.Create((x, y) =>
              {
                var curUser = HttpContext.Current.User;
                if (curUser != null)
                {
                  var curId = curUser.Identity.GetUserId();
                  if (x == curId)
                    return -1;
                  else if (y == curId)
                    return 1;
                }
                return string.Compare(x, y);
              });
    return _IdComparer;
  }
}

Is this safe? Any corrections or enhancements?

GregRos · Accepted Answer · 2015-03-15T01:58:37.580

Generating the comparer definitely costs more than storing it. It's a heap allocation, and more than one (you have to allocate the auto-generated class for the lambda).

You probably shouldn't worry about it though. The overhead is very small.

Your edit is fine. You don't even need to use a lock or checking for null. The assignment operation is guaranteed to be atomic. In the worst case you just create the same comparer twice.

By initializer below I mean:

    static readonly IComparer<string> _IdComparer = Comparer<string>.Create((x, y) => {
        var curUser = HttpContext.Current.User;
        if (curUser != null) {
            var curId = curUser.Identity.GetUserId();
            if (x == curId)
                return -1;
            else if (y == curId)
                return 1;
        }
        return string.Compare(x, y);
    });

    public static IComparer<string> IdComparer {
        get {
            return _IdComparer;
        }
    }

I don't really understand how you can be unaware of the initializer.

Yeah. Or you can initialize it using the initializer. It really, really doesn't matter. — GregRos, Mar 15 '15 at 01:43

Should I cache a comparer in a static field?

1 Answers1