[CHANGE] EthereumAddress: Deprecate value. Compare by converting to number
#314
Conversation
9c3bef3 to
4b6cb93
Compare
number This will avoid any string comparison issues. Also the string value shouldn't be exposed so explicity, created different converison methods in the type.
4b6cb93 to
8c1ec57
Compare
| public var value: String { | ||
| raw | ||
| } | ||
| private let raw: String |
There was a problem hiding this comment.
Did you consider making EthereumAddress conform to RawRepresentable<String>? That would give you a lot of the implementation for free.
You would only have to override the implementation for Hashable and Equatable.
There was a problem hiding this comment.
Just checked and it will require to expose rawValue as public. We really don't want to do that, and only expose formatting methods to force the caller to decide to which format to convert when getting the address
| public static let zero: Self = "0x0000000000000000000000000000000000000000" | ||
|
|
||
| public init(_ value: String) { | ||
| self.value = value.lowercased() | ||
| self.raw = value.lowercased() |
There was a problem hiding this comment.
Do we need .lowercased() ? Same question for the decode implementation. If we used RawRepresentable<String> the rawValue would always maintain its initial case.
There was a problem hiding this comment.
keeping the assumptions as before. I suspect that lowercased() was introduced when there were issues with comparision, which shouldn't be neede anymore. But I want to keep the same behaviour as before for now.
This will avoid any string comparison issues. Also the string value
shouldn't be exposed so explicity, created different converison methods
in the type.