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().

Phrancis
20.5k6 gold badges69 silver badges155 bronze badges
asked Mar 13, 2017 at 15:30
\$\endgroup\$
2
  • 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\$ Commented Mar 13, 2017 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\$ Commented Mar 14, 2017 at 8:06

4 Answers 4

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.

answered Mar 13, 2017 at 17:13
\$\endgroup\$
3
  • \$\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\$ Commented Mar 13, 2017 at 19:12
  • \$\begingroup\$ No problem. I know how it works. \$\endgroup\$ Commented Mar 13, 2017 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\$ Commented Mar 14, 2017 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);
answered Mar 13, 2017 at 19:19
\$\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

answered Mar 13, 2017 at 16:34
\$\endgroup\$
4
  • \$\begingroup\$ Great link to Microsofts explanation! Thanks. I'm a bit hesitant though about having a method named "Equals2". \$\endgroup\$ Commented Mar 14, 2017 at 7:03
  • \$\begingroup\$ You really think it must be named "Equals2"? \$\endgroup\$ Commented Mar 14, 2017 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\$ Commented Mar 14, 2017 at 8:30
  • \$\begingroup\$ Wow that is a tough one. How to name a method is hard for me too. \$\endgroup\$ Commented Mar 14, 2017 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.

answered Mar 14, 2017 at 9:21
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.