New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SR-10048] NSRange.init?(_ string: String) should either be internal or named to reflect its purpose #3521
Comments
This just needs docs. It is the moral replacement to NSRangeFromString |
I think it needs more than a documentation change. It needs a clarifying argument label. We should deprecate and replace it with an init that takes one. |
We don't have an argument label for going from Int to String either. This is a LosslessStringConvertible sort of thing. |
Comment by Ben Pious (JIRA) I agree that a clarifying label would be nice. @belkadan I think this is distinct from the similar function you mentioned in several respects: This function does not fulfill the same promises as LosslessStringConvertible: that you can go from A -> B -> A without ambiguity. `NSRange(NSRange(location: -1, length: 4).description) == NSRange(location: 1, length: 4)`. The negative sign is lost. Perhaps that's just a bug, but still notable imo. This is also far more liberal about what it accepts than Int?(_: String), for example. "3a" can't be converted to an Int by that function, but "3 a 4" can be converted to an NSRange. Incidentally, this function doesn't match the behavior of NSRangeFromString in an important respect: it rejects Strings like "1", while according to the documentation of NSRangeFromString this should be equivalent to calling NSRange(1, 0). Has any consideration been given to just removing this? I'm actually quite curious what the rationale for adding this to the original Core Foundation type was, since it seems like a bad idea to have something that's this liberal about what it accepts. |
The original API proposal that went through the internal Cocoa API review boiled down to: this is a safer alternative than the free floating function that did the same thing (NSRangeFromString) which definitely has consumers for archival. The C implementation would annoyingly throw NSExceptions and I found it much more ergonomic to use a nullable return instead. Changing this could be viewed as unproductive churn especially since we are advocating using Range instead since it has a better generic story than NSRange especially when it comes to strings. That all being said archival is important for end user data like document formats even if they are “legacy” so I am hesitant to advocate removing this entirely and pushing developers to bridge out to objc just to handle stringified NSRanges. In short, if we get a substantial glut of refinements for Foundation then I could see this being lumped in with those as a internal API proposal but just on its own it seems like small pickings. |
Environment
Xcode 10.1.
Additional Detail from JIRA
md5: e1582cb13c604afacfd80b0d7cf9f719
Issue Description:
Based on the function signature, a caller might reasonably expect [this function|https://github.com/apple/swift-corelibs-foundation/blob/master/Foundation/NSRange.swift#L150|http://example.com] to take a string, and return a range that encompasses the entire string. It returning nil seems a little strange, but it seems reasonable that it might return nil if the string is empty.
In fact, this initializer does something related to finding decimal numbers?
This init shows up in autocomplete in Xcode, and can create confusion for users. It should either be private, have a name that makes sense, have documentation, or some combination of the three.
The text was updated successfully, but these errors were encountered: