Merge pull request #2 from daehn/fix/hash-collisions

Fix hash collisions

CountedSet.xcodeproj/project.pbxproj

@@ -11,6 +11,7 @@
 		AF4300B01C9C980800D26703 /* CountedSet.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = AF4300A51C9C980700D26703 /* CountedSet.framework */; };
 		AF4300B51C9C980800D26703 /* CountedSetTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AF4300B41C9C980800D26703 /* CountedSetTests.swift */; };
 		AF4300C01C9C981300D26703 /* CountedSet.swift in Sources */ = {isa = PBXBuildFile; fileRef = AF4300BF1C9C981300D26703 /* CountedSet.swift */; };
+		AF441F1E1E4900F300DD008C /* HashHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = AF441F1D1E4900F300DD008C /* HashHelper.swift */; };
 /* End PBXBuildFile section */
 
 /* Begin PBXContainerItemProxy section */
@@ -31,6 +32,7 @@
 		AF4300B41C9C980800D26703 /* CountedSetTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CountedSetTests.swift; sourceTree = "<group>"; };
 		AF4300B61C9C980800D26703 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
 		AF4300BF1C9C981300D26703 /* CountedSet.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CountedSet.swift; sourceTree = "<group>"; };
+		AF441F1D1E4900F300DD008C /* HashHelper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HashHelper.swift; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -84,6 +86,7 @@
 			isa = PBXGroup;
 			children = (
 				AF4300B41C9C980800D26703 /* CountedSetTests.swift */,
+				AF441F1D1E4900F300DD008C /* HashHelper.swift */,
 				AF4300B61C9C980800D26703 /* Info.plist */,
 			);
 			path = CountedSetTests;
@@ -208,6 +211,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				AF4300B51C9C980800D26703 /* CountedSetTests.swift in Sources */,
+				AF441F1E1E4900F300DD008C /* HashHelper.swift in Sources */,
 			);
 			runOnlyForDeploymentPostprocessing = 0;
 		};

CountedSet/CountedSet.swift

@@ -27,24 +27,24 @@ public struct CountedSet<Element : Hashable> : ExpressibleByArrayLiteral {
     public typealias Index = SetIndex<Element>
 
     fileprivate var backing = Set<Element>()
-    fileprivate var countByHash = [Int: UInt]()
+    fileprivate var countByElement = [Element: UInt]()
 
     public mutating func insert(_ member: Element) {
         backing.insert(member)
-        let count = countByHash[member.hashValue] ?? 0
-        countByHash[member.hashValue] = count + 1
+        let count = countByElement[member] ?? 0
+        countByElement[member] = count + 1
     }
 
     @discardableResult public mutating func remove(_ member: Element) -> Element? {
-        guard var count = countByHash[member.hashValue], count > 0 else { return nil }
+        guard var count = countByElement[member], count > 0 else { return nil }
         count -= 1
-        countByHash[member.hashValue] = Swift.max(count, 0)
+        countByElement[member] = Swift.max(count, 0)
         if count <= 0 { backing.remove(member) }
         return member
     }
 
     public func count(for member: Element) -> UInt? {
-        return countByHash[member.hashValue]
+        return countByElement[member]
     }
 
     public init(arrayLiteral elements: Element...) {
@@ -96,15 +96,15 @@ extension CountedSet: Collection {
 extension CountedSet: Hashable {
 
     public var hashValue: Int {
-        return backing.hashValue ^ Int(countByHash.values.reduce(0, ^))
+        return backing.hashValue ^ Int(countByElement.values.reduce(0, ^))
     }
 
 }
 
 // MARK: - Equatable Operator
 
 public func ==<Element>(lhs: CountedSet<Element>, rhs: CountedSet<Element>) -> Bool {
-    return lhs.backing == rhs.backing && lhs.countByHash == rhs.countByHash
+    return lhs.backing == rhs.backing && lhs.countByElement == rhs.countByElement
 }
 
 // MARK: - CustomStringConvertible

CountedSetTests/CountedSetTests.swift

@@ -201,6 +201,45 @@ class CountedSetTests: XCTestCase {
         XCTAssertEqual(sut["Baz"], 1)
     }
 
+    func testThatItDoesNotAddTheCountsForElementsWithTheSameHash() {
+        // given
+        let first = HashHelper(hashValue: 1, name: "first")
+        let second = HashHelper(hashValue: 2, name: "second")
+
+        var sut = CountedSet(arrayLiteral: first, second)
+
+        // then
+        XCTAssertEqual(sut.count(for: first), 1)
+        XCTAssertEqual(sut.count(for: second), 1)
+
+        // when
+        let third = HashHelper(hashValue: 1, name: "third")
+        sut.insert(third)
+
+        // then
+        XCTAssertEqual(sut.count(for: first), 1)
+        XCTAssertEqual(sut.count(for: second), 1)
+        XCTAssertEqual(sut.count(for: third), 1)
+    }
+
+    func testThatItDoesAddTheCountsForElementsWithTheSameIdentity() {
+        // given
+        let first = HashHelper(hashValue: 1, name: "first")
+
+        var sut = CountedSet(arrayLiteral: first)
+
+        // then
+        XCTAssertEqual(sut.count(for: first), 1)
+
+        // when
+        let second = HashHelper(hashValue: 1, name: "first")
+        sut.insert(second)
+
+        // then
+        XCTAssertEqual(sut.count(for: first), 2)
+        XCTAssertEqual(sut.count(for: second), 2)
+    }
+
     func testThatItReturnsNilIfTheElementIsNotInTheSetWhenSubscipted() {
         // given
         sut = CountedSet(arrayLiteral: "Foo", "Bar")

CountedSetTests/HashHelper.swift

@@ -0,0 +1,42 @@
+//
+// The MIT License (MIT)
+// Copyright 2016 Silvan Dähn
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
+// of the Software, and to permit persons to whom the Software is furnished to do so,
+// subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
+// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+// HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
+// CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE
+// OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+//
+
+
+class HashHelper: Hashable, Equatable {
+
+    fileprivate let _hashValue: Int
+    fileprivate let name: String
+
+    init(hashValue: Int, name: String) {
+        _hashValue = hashValue
+        self.name = name
+    }
+
+    var hashValue: Int {
+        return _hashValue
+    }
+
+}
+
+func ==(lhs: HashHelper, rhs: HashHelper) -> Bool {
+    return lhs._hashValue == rhs._hashValue && lhs.name == rhs.name
+}