From 90fe75858248efc2bb3b5ebd95138bc39ca50798 Mon Sep 17 00:00:00 2001 From: Klaus Kneupner Date: Tue, 7 Jan 2025 22:39:37 +0100 Subject: [PATCH 1/6] removal of potentially harmful parameter. I managed to build an inconsistent graph as the edge.directed information was overwritten by addEdge(_, directed:). Now, the directed parameter is no longer needed. Instead, the information is taken from the Edge parameter. --- Sources/SwiftGraph/Graph.swift | 9 +++------ Sources/SwiftGraph/Reversed.swift | 2 +- Sources/SwiftGraph/Union.swift | 2 +- Sources/SwiftGraph/UniqueElementsGraph.swift | 7 ++----- Sources/SwiftGraph/UnweightedGraph.swift | 11 ++++------- Sources/SwiftGraph/WeightedGraph.swift | 9 +++------ Tests/SwiftGraphTests/SwiftGraphTests.swift | 2 +- 7 files changed, 15 insertions(+), 27 deletions(-) diff --git a/Sources/SwiftGraph/Graph.swift b/Sources/SwiftGraph/Graph.swift index 575d6fe..1d48ce8 100644 --- a/Sources/SwiftGraph/Graph.swift +++ b/Sources/SwiftGraph/Graph.swift @@ -26,7 +26,7 @@ public protocol Graph: CustomStringConvertible, Collection, Codable { var edges: [[E]] { get set } init(vertices: [V]) - func addEdge(_ e: E, directed: Bool) + func addEdge(_ e: E) } extension Graph { @@ -137,12 +137,9 @@ extension Graph { /// Add an edge to the graph. /// /// - parameter e: The edge to add. - /// - parameter directed: If false, undirected edges are created. - /// If true, a reversed edge is also created. - /// Default is false. - public mutating func addEdge(_ e: E, directed: Bool = false) { + public mutating func addEdge(_ e: E) { edges[e.u].append(e) - if !directed && e.u != e.v { + if !e.directed && e.u != e.v { edges[e.v].append(e.reversed()) } } diff --git a/Sources/SwiftGraph/Reversed.swift b/Sources/SwiftGraph/Reversed.swift index a91350f..9fa86e0 100644 --- a/Sources/SwiftGraph/Reversed.swift +++ b/Sources/SwiftGraph/Reversed.swift @@ -25,7 +25,7 @@ extension Graph { public func reversed() -> Self { let g = Self(vertices: self.vertices) for e in self.edgeList() { - g.addEdge(e.reversed(), directed: e.directed) + g.addEdge(e.reversed()) } return g } diff --git a/Sources/SwiftGraph/Union.swift b/Sources/SwiftGraph/Union.swift index 4af77c2..71bcff3 100644 --- a/Sources/SwiftGraph/Union.swift +++ b/Sources/SwiftGraph/Union.swift @@ -43,7 +43,7 @@ public extension UniqueElementsGraph where E == UnweightedEdge { // When vertices are removed from Graph, edges might mutate, // so we need to add new copies of them for the result graph. for edge in firstGraph.edges.joined() { - union.addEdge(edge, directed: true) + union.addEdge(edge) } for g in others { diff --git a/Sources/SwiftGraph/UniqueElementsGraph.swift b/Sources/SwiftGraph/UniqueElementsGraph.swift index 3ae8a64..5202c9c 100644 --- a/Sources/SwiftGraph/UniqueElementsGraph.swift +++ b/Sources/SwiftGraph/UniqueElementsGraph.swift @@ -50,14 +50,11 @@ open class UniqueElementsGraph: Gra /// Add an edge to the graph. Only allow the edge to be added once /// /// - parameter e: The edge to add. - /// - parameter directed: If false, undirected edges are created. - /// If true, a reversed edge is also created. - /// Default is false. - public func addEdge(_ e: E, directed: Bool = false) { + public func addEdge(_ e: E) { if !self.edgeExists(e) { edges[e.u].append(e) } - if !directed { + if !e.directed { let reversedEdge = e.reversed() if !edgeExists(reversedEdge) { edges[e.v].append(reversedEdge) diff --git a/Sources/SwiftGraph/UnweightedGraph.swift b/Sources/SwiftGraph/UnweightedGraph.swift index 9bedc44..caef9bc 100644 --- a/Sources/SwiftGraph/UnweightedGraph.swift +++ b/Sources/SwiftGraph/UnweightedGraph.swift @@ -33,12 +33,9 @@ open class UnweightedGraph: Graph { /// Add an edge to the graph. /// /// - parameter e: The edge to add. - /// - parameter directed: If false, undirected edges are created. - /// If true, a reversed edge is also created. - /// Default is false. - public func addEdge(_ e: UnweightedEdge, directed: Bool) { + public func addEdge(_ e: UnweightedEdge) { edges[e.u].append(e) - if !directed && e.u != e.v { + if !e.directed && e.u != e.v { edges[e.v].append(e.reversed()) } } @@ -112,7 +109,7 @@ extension Graph where E == UnweightedEdge { /// - parameter to: The ending vertex's index. /// - parameter directed: Is the edge directed? (default `false`) public func addEdge(fromIndex: Int, toIndex: Int, directed: Bool = false) { - addEdge(UnweightedEdge(u: fromIndex, v: toIndex, directed: directed), directed: directed) + addEdge(UnweightedEdge(u: fromIndex, v: toIndex, directed: directed)) } /// This is a convenience method that adds an unweighted, undirected edge between the first occurence of two vertices. It takes O(n) time. @@ -122,7 +119,7 @@ extension Graph where E == UnweightedEdge { /// - parameter directed: Is the edge directed? (default `false`) public func addEdge(from: V, to: V, directed: Bool = false) { if let u = indexOfVertex(from), let v = indexOfVertex(to) { - addEdge(UnweightedEdge(u: u, v: v, directed: directed), directed: directed) + addEdge(UnweightedEdge(u: u, v: v, directed: directed)) } } diff --git a/Sources/SwiftGraph/WeightedGraph.swift b/Sources/SwiftGraph/WeightedGraph.swift index ca0a728..5ed3c5d 100644 --- a/Sources/SwiftGraph/WeightedGraph.swift +++ b/Sources/SwiftGraph/WeightedGraph.swift @@ -34,12 +34,9 @@ open class WeightedGraph: Graph /// Add an edge to the graph. /// /// - parameter e: The edge to add. - /// - parameter directed: If false, undirected edges are created. - /// If true, a reversed edge is also created. - /// Default is false. - public func addEdge(_ e: WeightedEdge, directed: Bool) { + public func addEdge(_ e: WeightedEdge) { edges[e.u].append(e) - if !directed && e.u != e.v { + if !e.directed && e.u != e.v { edges[e.v].append(e.reversed()) } } @@ -77,7 +74,7 @@ extension Graph where E: WeightedEdgeProtocol { /// - parameter directed: Is the edge directed? (default false) /// - parameter weight: the Weight of the edge to add. public func addEdge(fromIndex: Int, toIndex: Int, weight: W, directed: Bool = false) { - addEdge(E(u: fromIndex, v: toIndex, directed: directed, weight: weight), directed: directed) + addEdge(E(u: fromIndex, v: toIndex, directed: directed, weight: weight)) } /// This is a convenience method that adds a weighted edge between the first occurence of two vertices. It takes O(n) time. diff --git a/Tests/SwiftGraphTests/SwiftGraphTests.swift b/Tests/SwiftGraphTests/SwiftGraphTests.swift index 55a28ca..09cf198 100644 --- a/Tests/SwiftGraphTests/SwiftGraphTests.swift +++ b/Tests/SwiftGraphTests/SwiftGraphTests.swift @@ -81,7 +81,7 @@ class SwiftGraphTests: XCTestCase { let arezzoIndex = g.addVertex("Arezzo") g.addEdge(from: "Atlanta", to: "New York", directed: true) let nyAtlantaEdge = UnweightedEdge(u: nyIndex, v: atlantaIndex, directed: true) - g.addEdge(nyAtlantaEdge, directed: true) + g.addEdge(nyAtlantaEdge) g.addEdge(from: "Miami", to: "Atlanta", directed: true) g.addEdge(from: "New York", to: "Miami", directed: false) g.addEdge(from: "Atlanta", to: "Miami", directed: true) From ce4996e40372356e4ecfe605a2ac24f0cad21b1a Mon Sep 17 00:00:00 2001 From: Klaus Kneupner Date: Wed, 8 Jan 2025 10:37:43 +0100 Subject: [PATCH 2/6] making it a non-breaking change --- Sources/SwiftGraph/Graph.swift | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Sources/SwiftGraph/Graph.swift b/Sources/SwiftGraph/Graph.swift index 1d48ce8..037d5ca 100644 --- a/Sources/SwiftGraph/Graph.swift +++ b/Sources/SwiftGraph/Graph.swift @@ -39,6 +39,11 @@ extension Graph { public var edgeCount: Int { return edges.joined().count } + + @available(*, deprecated, renamed: "addEdge", message: "Use the addEdge method without the additional directed parameter instead, as the Edge contains already the information about direction. A double specification can only result in inconsistencies and errors.") + func addEdge(_ e: E, directed: Bool = false){ + addEdge(e) + } /// Returns a list of all the edges, undirected edges are only appended once. public func edgeList() -> [E] { From 941dea625652bdc89e6f707081490af94c49aed7 Mon Sep 17 00:00:00 2001 From: Klaus Kneupner Date: Wed, 8 Jan 2025 11:13:58 +0100 Subject: [PATCH 3/6] small improvement, taking default value out. --- Sources/SwiftGraph/Graph.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftGraph/Graph.swift b/Sources/SwiftGraph/Graph.swift index 037d5ca..cd672c6 100644 --- a/Sources/SwiftGraph/Graph.swift +++ b/Sources/SwiftGraph/Graph.swift @@ -41,7 +41,7 @@ extension Graph { } @available(*, deprecated, renamed: "addEdge", message: "Use the addEdge method without the additional directed parameter instead, as the Edge contains already the information about direction. A double specification can only result in inconsistencies and errors.") - func addEdge(_ e: E, directed: Bool = false){ + func addEdge(_ e: E, directed: Bool){ addEdge(e) } From ad7a2ebb6d173d9f718e8503f46ccd46838d5220 Mon Sep 17 00:00:00 2001 From: Vithanco Date: Wed, 8 Jan 2025 17:06:13 +0100 Subject: [PATCH 4/6] Update Sources/SwiftGraph/Graph.swift Co-authored-by: Zev Eisenberg --- Sources/SwiftGraph/Graph.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftGraph/Graph.swift b/Sources/SwiftGraph/Graph.swift index cd672c6..0b210a2 100644 --- a/Sources/SwiftGraph/Graph.swift +++ b/Sources/SwiftGraph/Graph.swift @@ -40,7 +40,7 @@ extension Graph { return edges.joined().count } - @available(*, deprecated, renamed: "addEdge", message: "Use the addEdge method without the additional directed parameter instead, as the Edge contains already the information about direction. A double specification can only result in inconsistencies and errors.") + @available(*, deprecated, renamed: "addEdge(_:)", message: "Use the 'addEdge' method without the additional 'directed' parameter instead, as the Edge already contains the information about direction.") func addEdge(_ e: E, directed: Bool){ addEdge(e) } From 43cca35539a05a3a2c37bda136bc30fea96b131e Mon Sep 17 00:00:00 2001 From: Klaus Kneupner Date: Thu, 9 Jan 2025 05:40:51 +0100 Subject: [PATCH 5/6] formatting only --- Sources/SwiftGraph/Graph.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftGraph/Graph.swift b/Sources/SwiftGraph/Graph.swift index cd672c6..b1dbc7a 100644 --- a/Sources/SwiftGraph/Graph.swift +++ b/Sources/SwiftGraph/Graph.swift @@ -41,7 +41,7 @@ extension Graph { } @available(*, deprecated, renamed: "addEdge", message: "Use the addEdge method without the additional directed parameter instead, as the Edge contains already the information about direction. A double specification can only result in inconsistencies and errors.") - func addEdge(_ e: E, directed: Bool){ + func addEdge(_ e: E, directed: Bool) { addEdge(e) } From 970a0b8a8244679fda476bcdb84b5c46492e0ac6 Mon Sep 17 00:00:00 2001 From: Klaus Kneupner Date: Sun, 23 Mar 2025 16:37:15 +0100 Subject: [PATCH 6/6] removing not needed import --- Sources/SwiftGraph/Reversed.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/SwiftGraph/Reversed.swift b/Sources/SwiftGraph/Reversed.swift index 9fa86e0..8f56d15 100644 --- a/Sources/SwiftGraph/Reversed.swift +++ b/Sources/SwiftGraph/Reversed.swift @@ -16,8 +16,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import Foundation - extension Graph { /// Returns a graph of the same type with all edges reversed. ///