Better equality code

SabreTools.Library/DatFiles/ItemDictionary.cs

@@ -583,13 +583,10 @@ namespace SabreTools.Library.DatFiles
                 {
                     string key = oldkeys[k];
 
-                    // Get the unsorted current list
-                    List<DatItem> items = this[key];
-
                     // Now add each of the roms to their respective keys
-                    for (int i = 0; i < items.Count; i++)
+                    for (int i = 0; i < this[key].Count; i++)
                     {
-                        DatItem item = items[i];
+                        DatItem item = this[key][i];
                         if (item == null)
                             continue;
 

SabreTools.Library/DatItems/DatItem.cs

@@ -1106,6 +1106,26 @@ namespace SabreTools.Library.DatItems
 
         #region Sorting and Merging
 
+        /// <summary>
+        /// Determine if two hashes are equal for the purposes of merging
+        /// </summary>
+        /// <param name="firstHash">First hash to compare</param>
+        /// <param name="secondHash">Second hash to compare</param>
+        /// <returns>True if either is empty OR the hashes exactly match, false otherwise</returns>
+        public static bool ConditionalHashEquals(byte[] firstHash, byte[] secondHash)
+        {
+            // If either hash is empty, we say they're equal for merging
+            if (firstHash.IsNullOrEmpty() || secondHash.IsNullOrEmpty())
+                return true;
+
+            // If they're different sizes, they can't match
+            if (firstHash.Length != secondHash.Length)
+                return false;
+
+            // Otherwise, they need to match exactly
+            return Enumerable.SequenceEqual(firstHash, secondHash);
+        }
+
         /// <summary>
         /// Merge an arbitrary set of ROMs based on the supplied information
         /// </summary>

SabreTools.Library/DatItems/Disk.cs

@@ -1,6 +1,5 @@
 using System.Linq;
 
-using SabreTools.Library.Data;
 using SabreTools.Library.FileTypes;
 using SabreTools.Library.Tools;
 using Newtonsoft.Json;
@@ -282,50 +281,23 @@ namespace SabreTools.Library.DatItems
         {
             bool dupefound = false;
 
-            // If we don't have a disk, return false
+            // If we don't have a rom, return false
-            if (this.ItemType != other.ItemType)
+            if (ItemType != other.ItemType)
                 return dupefound;
 
             // Otherwise, treat it as a Disk
             Disk newOther = other as Disk;
 
             // If all hashes are empty but they're both nodump and the names match, then they're dupes
-            if ((this.ItemStatus == ItemStatus.Nodump && newOther.ItemStatus == ItemStatus.Nodump)
+            if ((ItemStatus == ItemStatus.Nodump && newOther.ItemStatus == ItemStatus.Nodump)
-                && (this.Name == newOther.Name)
+                && Name == newOther.Name
-                && (this._md5.IsNullOrEmpty() && newOther._md5.IsNullOrEmpty())
+                && !HasHashes() && !newOther.HasHashes())
-#if NET_FRAMEWORK
-                && (this._ripemd160.IsNullOrEmpty() && newOther._ripemd160.IsNullOrEmpty())
-#endif
-                && (this._sha1.IsNullOrEmpty() && newOther._sha1.IsNullOrEmpty())
-                && (this._sha256.IsNullOrEmpty() && newOther._sha256.IsNullOrEmpty())
-                && (this._sha384.IsNullOrEmpty() && newOther._sha384.IsNullOrEmpty())
-                && (this._sha512.IsNullOrEmpty() && newOther._sha512.IsNullOrEmpty()))
             {
                 dupefound = true;
             }
 
-            // If we can determine that the disks have no non-empty hashes in common, we return false
-            else if ((this._md5.IsNullOrEmpty() || newOther._md5.IsNullOrEmpty())
-#if NET_FRAMEWORK
-                && (this._ripemd160.IsNullOrEmpty() || newOther._ripemd160.IsNullOrEmpty())
-#endif
-                && (this._sha1.IsNullOrEmpty() || newOther._sha1.IsNullOrEmpty())
-                && (this._sha256.IsNullOrEmpty() || newOther._sha256.IsNullOrEmpty())
-                && (this._sha384.IsNullOrEmpty() || newOther._sha384.IsNullOrEmpty())
-                && (this._sha512.IsNullOrEmpty() || newOther._sha512.IsNullOrEmpty()))
-            {
-                dupefound = false;
-            }
-
             // Otherwise if we get a partial match
-            else if (((this._md5.IsNullOrEmpty() || newOther._md5.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._md5, newOther._md5))
+            else if (HashMatch(newOther))
-#if NET_FRAMEWORK
-                && ((this._ripemd160.IsNullOrEmpty() || newOther._ripemd160.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._ripemd160, newOther._ripemd160))
-#endif
-                && ((this._sha1.IsNullOrEmpty() || newOther._sha1.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha1, newOther._sha1))
-                && ((this._sha256.IsNullOrEmpty() || newOther._sha256.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha256, newOther._sha256))
-                && ((this._sha384.IsNullOrEmpty() || newOther._sha384.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha384, newOther._sha384))
-                && ((this._sha512.IsNullOrEmpty() || newOther._sha512.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha512, newOther._sha512)))
             {
                 dupefound = true;
             }
@@ -333,6 +305,65 @@ namespace SabreTools.Library.DatItems
             return dupefound;
         }
 
+        /// <summary>
+        /// Returns if there are no, non-empty hashes in common with another Disk
+        /// </summary>
+        /// <param name="other">Disk to compare against</param>
+        /// <returns>True if at least one hash is not mutually exclusive, false otherwise</returns>
+        private bool HasCommonHash(Disk other)
+        {
+            return !(_md5.IsNullOrEmpty() ^ other._md5.IsNullOrEmpty())
+#if NET_FRAMEWORK
+                || !(_ripemd160.IsNullOrEmpty() || other._ripemd160.IsNullOrEmpty())
+#endif
+                || !(_sha1.IsNullOrEmpty() ^ other._sha1.IsNullOrEmpty())
+                || !(_sha256.IsNullOrEmpty() ^ other._sha256.IsNullOrEmpty())
+                || !(_sha384.IsNullOrEmpty() ^ other._sha384.IsNullOrEmpty())
+                || !(_sha512.IsNullOrEmpty() ^ other._sha512.IsNullOrEmpty());
+        }
+
+        /// <summary>
+        /// Returns if the Disk contains any hashes
+        /// </summary>
+        /// <returns>True if any hash exists, false otherwise</returns>
+        private bool HasHashes()
+        {
+            return !_md5.IsNullOrEmpty()
+#if NET_FRAMEWORK
+                || !_ripemd160.IsNullOrEmpty()
+#endif
+                || !_sha1.IsNullOrEmpty()
+                || !_sha256.IsNullOrEmpty()
+                || !_sha384.IsNullOrEmpty()
+                || !_sha512.IsNullOrEmpty();
+        }
+
+        /// <summary>
+        /// Returns if any hashes are common with another Disk
+        /// </summary>
+        /// <param name="other">Disk to compare against</param>
+        /// <returns>True if any hashes are in common, false otherwise</returns>
+        private bool HashMatch(Disk other)
+        {
+            // If either have no hashes, we return false, otherwise this would be a false positive
+            if (!HasHashes() || !other.HasHashes())
+                return false;
+
+            // If neither have hashes in common, we return false, otherwise this would be a false positive
+            if (!HasCommonHash(other))
+                return false;
+
+            // Return if all hashes match according to merge rules
+            return ConditionalHashEquals(_md5, other._md5)
+#if NET_FRAMEWORK
+                && ConditionalHashEquals(_ripemd160, other._ripemd160)
+#endif
+                && ConditionalHashEquals(_sha1, other._sha1)
+                && ConditionalHashEquals(_sha256, other._sha256)
+                && ConditionalHashEquals(_sha384, other._sha384)
+                && ConditionalHashEquals(_sha512, other._sha512);
+        }
+
         #endregion
     }
 }

SabreTools.Library/DatItems/Rom.cs

@@ -278,68 +278,28 @@ namespace SabreTools.Library.DatItems
             bool dupefound = false;
 
             // If we don't have a rom, return false
-            if (this.ItemType != other.ItemType)
+            if (ItemType != other.ItemType)
                 return dupefound;
 
             // Otherwise, treat it as a Rom
             Rom newOther = other as Rom;
 
             // If all hashes are empty but they're both nodump and the names match, then they're dupes
-            if ((this.ItemStatus == ItemStatus.Nodump && newOther.ItemStatus == ItemStatus.Nodump)
+            if ((ItemStatus == ItemStatus.Nodump && newOther.ItemStatus == ItemStatus.Nodump)
-                && (this.Name == newOther.Name)
+                && Name == newOther.Name
-                && (this._crc.IsNullOrEmpty() && newOther._crc.IsNullOrEmpty())
+                && !HasHashes() && !newOther.HasHashes())
-                && (this._md5.IsNullOrEmpty() && newOther._md5.IsNullOrEmpty())
-#if NET_FRAMEWORK
-                && (this._ripemd160.IsNullOrEmpty() && newOther._ripemd160.IsNullOrEmpty())
-#endif
-                && (this._sha1.IsNullOrEmpty() && newOther._sha1.IsNullOrEmpty())
-                && (this._sha256.IsNullOrEmpty() && newOther._sha256.IsNullOrEmpty())
-                && (this._sha384.IsNullOrEmpty() && newOther._sha384.IsNullOrEmpty())
-                && (this._sha512.IsNullOrEmpty() && newOther._sha512.IsNullOrEmpty()))
             {
                 dupefound = true;
             }
 
-            // If we can determine that the roms have no non-empty hashes in common, we return false
-            else if ((this._crc.IsNullOrEmpty() || newOther._crc.IsNullOrEmpty())
-                && (this._md5.IsNullOrEmpty() || newOther._md5.IsNullOrEmpty())
-#if NET_FRAMEWORK
-                && (this._ripemd160.IsNullOrEmpty() || newOther._ripemd160.IsNullOrEmpty())
-#endif
-                && (this._sha1.IsNullOrEmpty() || newOther._sha1.IsNullOrEmpty())
-                && (this._sha256.IsNullOrEmpty() || newOther._sha256.IsNullOrEmpty())
-                && (this._sha384.IsNullOrEmpty() || newOther._sha384.IsNullOrEmpty())
-                && (this._sha512.IsNullOrEmpty() || newOther._sha512.IsNullOrEmpty()))
-            {
-                dupefound = false;
-            }
-
             // If we have a file that has no known size, rely on the hashes only
-            else if ((this.Size == -1)
+            else if (Size == -1 && HashMatch(newOther))
-                && ((this._crc.IsNullOrEmpty() || newOther._crc.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._crc, newOther._crc))
-                && ((this._md5.IsNullOrEmpty() || newOther._md5.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._md5, newOther._md5))
-#if NET_FRAMEWORK
-                && ((this._ripemd160.IsNullOrEmpty() || newOther._ripemd160.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._ripemd160, newOther._ripemd160))
-#endif
-                && ((this._sha1.IsNullOrEmpty() || newOther._sha1.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha1, newOther._sha1))
-                && ((this._sha256.IsNullOrEmpty() || newOther._sha256.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha256, newOther._sha256))
-                && ((this._sha384.IsNullOrEmpty() || newOther._sha384.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha384, newOther._sha384))
-                && ((this._sha512.IsNullOrEmpty() || newOther._sha512.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha512, newOther._sha512)))
             {
                 dupefound = true;
             }
 
             // Otherwise if we get a partial match
-            else if ((this.Size == newOther.Size)
+            else if (Size == newOther.Size && HashMatch(newOther))
-                && ((this._crc.IsNullOrEmpty() || newOther._crc.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._crc, newOther._crc))
-                && ((this._md5.IsNullOrEmpty() || newOther._md5.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._md5, newOther._md5))
-#if NET_FRAMEWORK
-                && ((this._ripemd160.IsNullOrEmpty() || newOther._ripemd160.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._ripemd160, newOther._ripemd160))
-#endif
-                && ((this._sha1.IsNullOrEmpty() || newOther._sha1.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha1, newOther._sha1))
-                && ((this._sha256.IsNullOrEmpty() || newOther._sha256.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha256, newOther._sha256))
-                && ((this._sha384.IsNullOrEmpty() || newOther._sha384.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha384, newOther._sha384))
-                && ((this._sha512.IsNullOrEmpty() || newOther._sha512.IsNullOrEmpty()) || Enumerable.SequenceEqual(this._sha512, newOther._sha512)))
             {
                 dupefound = true;
             }
@@ -347,6 +307,68 @@ namespace SabreTools.Library.DatItems
             return dupefound;
         }
 
+        /// <summary>
+        /// Returns if there are no, non-empty hashes in common with another Rom
+        /// </summary>
+        /// <param name="other">Rom to compare against</param>
+        /// <returns>True if at least one hash is not mutually exclusive, false otherwise</returns>
+        private bool HasCommonHash(Rom other)
+        {
+            return !(_crc.IsNullOrEmpty() ^ other._crc.IsNullOrEmpty())
+                || !(_md5.IsNullOrEmpty() ^ other._md5.IsNullOrEmpty())
+#if NET_FRAMEWORK
+                || !(_ripemd160.IsNullOrEmpty() || other._ripemd160.IsNullOrEmpty())
+#endif
+                || !(_sha1.IsNullOrEmpty() ^ other._sha1.IsNullOrEmpty())
+                || !(_sha256.IsNullOrEmpty() ^ other._sha256.IsNullOrEmpty())
+                || !(_sha384.IsNullOrEmpty() ^ other._sha384.IsNullOrEmpty())
+                || !(_sha512.IsNullOrEmpty() ^ other._sha512.IsNullOrEmpty());
+        }
+
+        /// <summary>
+        /// Returns if the Rom contains any hashes
+        /// </summary>
+        /// <returns>True if any hash exists, false otherwise</returns>
+        private bool HasHashes()
+        {
+            return !_crc.IsNullOrEmpty()
+                || !_md5.IsNullOrEmpty()
+#if NET_FRAMEWORK
+                || !_ripemd160.IsNullOrEmpty()
+#endif
+                || !_sha1.IsNullOrEmpty()
+                || !_sha256.IsNullOrEmpty()
+                || !_sha384.IsNullOrEmpty()
+                || !_sha512.IsNullOrEmpty();
+        }
+
+        /// <summary>
+        /// Returns if any hashes are common with another Rom
+        /// </summary>
+        /// <param name="other">Rom to compare against</param>
+        /// <returns>True if any hashes are in common, false otherwise</returns>
+        private bool HashMatch(Rom other)
+        {
+            // If either have no hashes, we return false, otherwise this would be a false positive
+            if (!HasHashes() || !other.HasHashes())
+                return false;
+
+            // If neither have hashes in common, we return false, otherwise this would be a false positive
+            if (!HasCommonHash(other))
+                return false;
+
+            // Return if all hashes match according to merge rules
+            return ConditionalHashEquals(_crc, other._crc)
+                && ConditionalHashEquals(_md5, other._md5)
+#if NET_FRAMEWORK
+                && ConditionalHashEquals(_ripemd160, other._ripemd160)
+#endif
+                && ConditionalHashEquals(_sha1, other._sha1)
+                && ConditionalHashEquals(_sha256, other._sha256)
+                && ConditionalHashEquals(_sha384, other._sha384)
+                && ConditionalHashEquals(_sha512, other._sha512);
+        }
+
         #endregion
     }
 }

SabreTools.Library/Filtering/Filter.cs

@@ -821,23 +821,6 @@ namespace SabreTools.Library.Filtering
                 // Run internal splitting
                 ProcessSplitType(datFile, this.InternalSplit);
 
-                // We remove any blanks, if we aren't supposed to have any
-                if (!datFile.Header.KeepEmptyGames)
-                {
-                    List<string> possiblyEmptyKeys = datFile.Items.Keys.ToList();
-                    foreach (string key in possiblyEmptyKeys)
-                    {
-                        List<DatItem> items = datFile.Items[key];
-                        if (items == null)
-                            continue;
-
-                        List<DatItem> newitems = items.Where(i => i.ItemType != ItemType.Blank).ToList();
-
-                        datFile.Items.Remove(key);
-                        datFile.Items.AddRange(key, newitems);
-                    }
-                }
-
                 // Loop over every key in the dictionary
                 List<string> keys = datFile.Items.Keys.ToList();
                 foreach (string key in keys)
@@ -906,6 +889,26 @@ namespace SabreTools.Library.Filtering
                 // If we are removing fields, do that now
                 if (RemoveFields)
                     RemoveFieldsFromItems(datFile);
+
+                // We remove any blanks, if we aren't supposed to have any
+                if (!datFile.Header.KeepEmptyGames)
+                {
+                    List<string> possiblyEmptyKeys = datFile.Items.Keys.ToList();
+                    foreach (string key in possiblyEmptyKeys)
+                    {
+                        List<DatItem> items = datFile.Items[key];
+                        if (items == null || items.Count == 0)
+                        {
+                            datFile.Items.Remove(key);
+                            continue;
+                        }
+
+                        List<DatItem> newitems = items.Where(i => i.ItemType != ItemType.Blank).ToList();
+
+                        datFile.Items.Remove(key);
+                        datFile.Items.AddRange(key, newitems);
+                    }
+                }
             }
             catch (Exception ex)
             {