removal of potentially harmful parameter.#91
Conversation
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.
|
I should mention, that I only removed the parameter where the information is already provided via Edge. In other cases, like convience addEdge methods, it is kept. |
|
This is, unfortunately, a breaking change as I changed the Graph protocol. |
|
Thanks for the contribution. This does potentially make sense but yeah unfortunately is a breaking change. One way to make it a non-breaking change would be to have new one parameter methods for |
|
I think I managed to make it a non-breaking change for most use cases. The "only" time the code will now break is if someone else implemented the Graph protocol separately. |
ZevEisenberg
left a comment
ZevEisenberg
left a comment
There was a problem hiding this comment.
A couple of minor suggestions on the last commit, but this seems like a smart change, and the deprecation is a nice way to go about it.
Sources/SwiftGraph/Graph.swift
Outdated
| } | ||
|
|
||
| @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){ |
There was a problem hiding this comment.
| func addEdge(_ e: E, directed: Bool = false){ | |
| func addEdge(_ e: E, directed: Bool = false) { |
There was a problem hiding this comment.
actually, you found an outdated version. I removed the default value (false) in the latest version.
There was a problem hiding this comment.
Without the default value there is a more clear distinction between the one-parameter version and the 2-parameter. And there is no confusion that you use the one-parameter version when you leave out the default parameter - and not using the default value
There was a problem hiding this comment.
formatting updated.
Co-authored-by: Zev Eisenberg <zev@zeveisenberg.com>
# Conflicts: # Sources/SwiftGraph/Graph.swift
|
sorry, KneupnerTrackunit is my alterEgo. Not sure why the last commits where under that name. |
|
Did you think about this one? |
4 participants
I managed to build an inconsistent graph as the edge.directed information was overwritten by 2nd parameter of addEdge(_, directed:). Now, the directed parameter is no longer needed. Instead, the information is taken from the Edge parameter.
One source of potential problems less.