views:

140

answers:

2

I don't see any problems with this code, but it feels like I'm missing something. Maybe it is possible to reduce the number of lines. Or is there even a bug to be fixed? I'm open to any suggestions.

public class NameComparer : IEqualityComparer<FileInfo>
{
 public bool Equals (FileInfo x, FileInfo y)
 {
  if (x == null) {
   return y == null;
  }

  if (y == null) {
   return false; 
  }

  return x.Name.Equals (y.Name);
 }

 public int GetHashCode (FileInfo obj)
 {
  return obj.Name.GetHashCode ();
 }
}
A: 

You might want to consider whether you want a case-insensitive comparison.

Joe
+2  A: 

You should first return true if the FileInfo's equality operator returns true. Also, specify the type of string comparison you want to do. Presumably you'd want to ignore case, since these are filenames.

    public bool Equals(FileInfo x, FileInfo y)
    {
        if (x == y)
        {
            return true;
        }

        if (x == null || y == null)
        {
            return false;
        }

        return string.Compare(x.Name, y.Name, StringComparison.OrdinalIgnoreCase) == 0;
    }
AdamRalph
Do you really need to check for y == null after comparing x == y? Maybe second if should be like this: if (x == null || y == null) return false?
Dmitry
Thanks for pointing that out. Have edited.
AdamRalph