3
\$\begingroup\$

I'm unsure of how to Name and clearly distinguish between methods that deals with comparisons.

I know of the following two established concepts:

.Equals: A method to see if two objects refer to the same instance

.Compare: A method often used when determining sort order among objects

But what to name a method when you are interested to see if two different object instances refer to the same "logical" object, or when comparing values of two different object instances? Let me give you an example with a class named Photo.

public class Photo : IEquatable<Photo>
{
    public string FileName { get; private set; }
    public string InFolder { get; private set; }
    public string FullFilePath { get; private set; }
    public string FileExtension { get; private set; }
    public long FileSize { get; private set; }
    public DateTime FileLastModified { get; private set; }
    public DateTime FileCreated { get; private set; }
    public ExifData EXIF 
    {
        get
        {
            if (this._exif == null)
                LoadExtendedProperties();
            return this._exif;
        }
        set { this._exif = value; }
    }
    public Photo(string fullFilePath)
    {
        //...
    }
    public override int GetHashCode()
    {
        return this.FullFilePath.GetHashCode();
    }
    private void LoadExtendedProperties()
    {
        //...
    }
    public bool Equals(Photo other)
    {
        if (object.ReferenceEquals(this, null) || object.ReferenceEquals(other, null))
            return false;
        if (object.ReferenceEquals(this, other))
            return true;
        else
            return false;
    }
    /// <summary>
    /// Do they refer to the same photo (file)
    /// </summary>
    public bool UnnamedCompareMethodA(Photo other)
    {
        if(object.ReferenceEquals(other, null))
            return false;
        if (this.FullFilePath == other.FullFilePath)
            return true;
        else
            return false;
    }
    /// <summary>
    /// Do all properties have the same value?
    /// Usefull when determining if a Photo has been modified since last loaded
    /// </summary>  
    public bool UnnamedCompareMethodB(Photo other)
    {
        // Note: Instead of manually adding fields; Reflection might be a better choice.
        // Though that might bring other challenges as some fields should be left out when comparing.

        // If any object is null then they are NOT considered equal
        if (object.ReferenceEquals(this, null) || object.ReferenceEquals(other, null))
            return false;
        if (this.FileLastModified != other.FileLastModified)
            return false;
        if (this.FullFilePath != other.FullFilePath)
            return false;

        if (this.FileSize != other.FileSize)
            return false;
        if (this.EXIF.Title != other.EXIF.Title)
            return false;
        if (this.EXIF.Rating != other.EXIF.Rating)
            return false;
        if (this.EXIF.PictureTaken != this.EXIF.PictureTaken)
            return false;
        if (!this.EXIF.Orientation.Equals(other.EXIF.Orientation))
            return false;
        if (!this.EXIF.Dimensions.Equals(other.EXIF.Dimensions))
            return false;
        if (!this.EXIF.Location.Equals(other.EXIF.Location))
            return false;

        return true;
    }
}

Question: What to name UnnamedCompareMethodA() and UnnamedCompareMethodB()? I would like to have "Equal" somewhere in the name but I feel like that would create confusion as they are not related at all with .Equals().

\$\endgroup\$
  • 1
    \$\begingroup\$ It would have been nice to include the actual constructor code and LoadExtendedProperties implementation. CR answers typically touch on every aspect of the code. I presume your ctor only calls the private setters, and that LoadExtendedProperties can never throw an exception? \$\endgroup\$ – Mathieu Guindon Mar 13 '17 at 19:01
  • 1
    \$\begingroup\$ I have rolled back the last edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Phrancis Mar 14 '17 at 8:06
4
\$\begingroup\$

You seem confused. Given two objects, a.Equals(b) tests whether they have equivalent content. Basically, what you wrote as UnnamedCompareMethodB() should actually be the Equals() method.

I wouldn't bother with defining an UnnamedCompareMethodA() at all. If someone wants to compare objects by FullFilePath, they can just write a.FullFilePath.Equals(b.FullFilePath).

Furthermore, calling object.ReferenceEquals(this, null) makes little sense. If the object were indeed null, the code would have already crashed with a NullReferenceException before then.

\$\endgroup\$
  • \$\begingroup\$ @Paparazzi The rule is that if two objects are .Equals() to each other, then they must have the same GetHashCode(). Two unequal objects may share the same hash code (though that is discouraged). One can imagine instantiating var a = new Photo("image.jpg"), then modifying image.jpg on disk, then instantiating var b = new Photo("image.jpg"). I would then expect a.Equals(b) to be false, but I wouldn't consider it a tragedy if a and b shared the same hash code, as it's a bit of a corner case. \$\endgroup\$ – 200_success Mar 13 '17 at 19:12
  • \$\begingroup\$ No problem. I know how it works. \$\endgroup\$ – paparazzo Mar 13 '17 at 20:14
  • \$\begingroup\$ Its funny how things are obvious once someone explains. I changed my Equals() method according to your answer. And to determining if two Photo objects refer to the same file I check a.FullFilePath.Equals( b.FullFilePath). \$\endgroup\$ – David Mar 14 '17 at 7:51
4
\$\begingroup\$

I find a cleaner way for comparing objects is to implement is as a separate class with the IEqualityComparer<T> interface. With it there is no confusion between the various Equals methods and you separate the comparison logic from the object itself. Also instead of negating each condition you can chain them with && and use positive ones:

class PhotoComparer : IEqualityComparer<Photo>
{
    public bool Equals(Photo x, Photo y)
    {
        return 
            !ReferenceEquals(x, null) &&
            !ReferenceEquals(y, null) &&
            x.FileLastModified == y.FileLastModified &&
            x.FullFilePath == y.FullFilePath &&
            x.FileSize == y.FileSize &&
            x.EXIF.Title == y.EXIF.Title &&
            x.EXIF.Rating == y.EXIF.Rating &&
            x.EXIF.PictureTaken == y.EXIF.PictureTaken &&
            x.EXIF.Orientation.Equals(y.EXIF.Orientation) &&
            x.EXIF.Dimensions.Equals(y.EXIF.Dimensions) &&
            x.EXIF.Location.Equals(y.EXIF.Location);
    }

    public int GetHashCode(Photo obj)
    {
        return obj.FullFilePath.GetHashCode();
    }
}

You can use it for dictionaries, hash-sets, distinct and basically everything that allows you to specify an IEqualityComparer or just create an instance of it and use it without any collections:

var photoComparer = new PhotoComparer();
var areSame = photoComparer.Equals(pthoto1, pthoto2);
\$\endgroup\$
2
\$\begingroup\$

You are not correct

I know of the following two established concepts:

.Equals: A method to see if two objects refer to the same instance

Equals can be used to determine if two Objects are the same and not necessarily a reference to the same instance.

Microsoft has a good explanation

Right now below is calls to 3 different Equals
The last will be System Equals
I don't think that is your intent

//test lines start 
Photo cm1 = new Photo("alsdkf");
Photo cm2 = new Photo("alskdjf");
Debug.WriteLine(cm1.Equals(cm2));
Debug.WriteLine(cm1.Equals((object)cm2));
Debug.WriteLine(((object)cm1).Equals((object)cm2));
//test lines end

public class Photo 
{
    public bool Equals(Object obj)
    {
        return true;
    }
    public bool Equals(Photo obj)
    {
        return true;
    }
    public string FullFilePath { get; private set; }
    public Photo (string fullFilePath)
    {
        FullFilePath = fullFilePath;
    }
}

I think you are looking for something more like this

public class Photo2 : Object
{
    public override int GetHashCode()
    {
        return FullFilePath.GetHashCode();
    }
    public override bool Equals(Object obj)
    {
        if (obj == null || GetType() != obj.GetType())
            return false;

        Photo2 p = (Photo2)obj;
        return (FullFilePath == p.FullFilePath);
    }
    public bool Equals2(Photo obj)
    {
        return  true;  //sub in what ever you want 
    }
    public string FullFilePath { get; private set; }
    public Photo2(string fullFilePath)
    {
        FullFilePath = fullFilePath;
    }
}

Just call ReferenceEquals external to the Class
I don't think the class can even override ReferenceEquals

\$\endgroup\$
  • \$\begingroup\$ Great link to Microsofts explanation! Thanks. I'm a bit hesitant though about having a method named "Equals2". \$\endgroup\$ – David Mar 14 '17 at 7:03
  • \$\begingroup\$ You really think it must be named "Equals2"? \$\endgroup\$ – paparazzo Mar 14 '17 at 8:13
  • \$\begingroup\$ No :-). What would be a good name for .Equals2()? I feel it is hard to come up with a name that does not create confusion with .Equals. Maybe .HasTheSamePropertyValuesAs(Photo other) \$\endgroup\$ – David Mar 14 '17 at 8:30
  • \$\begingroup\$ Wow that is a tough one. How to name a method is hard for me too. \$\endgroup\$ – paparazzo Mar 14 '17 at 8:46
2
\$\begingroup\$
    /// <summary>
    /// Do all properties have the same value?
    /// Usefull when determining if a Photo has been modified since last loaded
    /// </summary>  

Perhaps the problem is at a higher level. Extrapolating from the information available, you seem to be making a photo management system with the ability to edit EXIF metadata. The key question I have is: does it make sense for there to be two Photo instances with the same path but different EXIF data? I wonder whether you might not be better off using some kind of lightweight pattern to ensure that at any one time there is at most one Photo instance per path.

That aside...


    public string FileName { get; private set; }
    public string InFolder { get; private set; }
    public string FullFilePath { get; private set; }
    public string FileExtension { get; private set; }

How many of these are independent? I wonder whether you should have one (or at the most two) properties here with a private setter and the rest should use System.IO.Path and derive their values from the primary one.


        if (this.EXIF.Title != other.EXIF.Title)
            return false;
        if (this.EXIF.Rating != other.EXIF.Rating)
            return false;
        if (this.EXIF.PictureTaken != this.EXIF.PictureTaken)
            return false;
        if (!this.EXIF.Orientation.Equals(other.EXIF.Orientation))
            return false;
        if (!this.EXIF.Dimensions.Equals(other.EXIF.Dimensions))
            return false;
        if (!this.EXIF.Location.Equals(other.EXIF.Location))
            return false;

Wrong place. This should be in ExifData.Equals.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.