Implementing `components(separatedBy:)` method in Swift
On looking at the components(separatedBy separator: String) -> [String]
method from the Swift Standard Library, I tried to come up with an implementation just for practice. Your comments are welcome to improve the same. Thanks.
Input:
func main() {
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: "."))
}
Output:
["Do not be sorry", " Be better", ""]
Implementation:
extension StringProtocol {
func components<T>(separatedBy separatorString: T) -> [String] where T: StringProtocol {
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
forEach { (character) in
if String(character) == separatorString {
separatedStrings.append(stringBuffer); stringBuffer = ""
} else {
stringBuffer += .init(character)
}
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
currentIndex += 1
}
return separatedStrings
}
}
extension Collection {
var lastIndex:Int {
get {
return self.count - 1
}
}
}
algorithm swift complexity
add a comment |
On looking at the components(separatedBy separator: String) -> [String]
method from the Swift Standard Library, I tried to come up with an implementation just for practice. Your comments are welcome to improve the same. Thanks.
Input:
func main() {
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: "."))
}
Output:
["Do not be sorry", " Be better", ""]
Implementation:
extension StringProtocol {
func components<T>(separatedBy separatorString: T) -> [String] where T: StringProtocol {
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
forEach { (character) in
if String(character) == separatorString {
separatedStrings.append(stringBuffer); stringBuffer = ""
} else {
stringBuffer += .init(character)
}
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
currentIndex += 1
}
return separatedStrings
}
}
extension Collection {
var lastIndex:Int {
get {
return self.count - 1
}
}
}
algorithm swift complexity
Right off the bat I can see that the array returned could be named better asseparatedComponents
instead ofseparatedStrings
.
– Badhan Ganesh
Dec 13 at 22:05
Could you please point to the SL implementation ofcomponents(separatedBy separator: String) -> [String]
?
– Carpsen90
Dec 16 at 13:15
add a comment |
On looking at the components(separatedBy separator: String) -> [String]
method from the Swift Standard Library, I tried to come up with an implementation just for practice. Your comments are welcome to improve the same. Thanks.
Input:
func main() {
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: "."))
}
Output:
["Do not be sorry", " Be better", ""]
Implementation:
extension StringProtocol {
func components<T>(separatedBy separatorString: T) -> [String] where T: StringProtocol {
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
forEach { (character) in
if String(character) == separatorString {
separatedStrings.append(stringBuffer); stringBuffer = ""
} else {
stringBuffer += .init(character)
}
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
currentIndex += 1
}
return separatedStrings
}
}
extension Collection {
var lastIndex:Int {
get {
return self.count - 1
}
}
}
algorithm swift complexity
On looking at the components(separatedBy separator: String) -> [String]
method from the Swift Standard Library, I tried to come up with an implementation just for practice. Your comments are welcome to improve the same. Thanks.
Input:
func main() {
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: "."))
}
Output:
["Do not be sorry", " Be better", ""]
Implementation:
extension StringProtocol {
func components<T>(separatedBy separatorString: T) -> [String] where T: StringProtocol {
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
forEach { (character) in
if String(character) == separatorString {
separatedStrings.append(stringBuffer); stringBuffer = ""
} else {
stringBuffer += .init(character)
}
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
currentIndex += 1
}
return separatedStrings
}
}
extension Collection {
var lastIndex:Int {
get {
return self.count - 1
}
}
}
algorithm swift complexity
algorithm swift complexity
asked Dec 13 at 21:54
Badhan Ganesh
1479
1479
Right off the bat I can see that the array returned could be named better asseparatedComponents
instead ofseparatedStrings
.
– Badhan Ganesh
Dec 13 at 22:05
Could you please point to the SL implementation ofcomponents(separatedBy separator: String) -> [String]
?
– Carpsen90
Dec 16 at 13:15
add a comment |
Right off the bat I can see that the array returned could be named better asseparatedComponents
instead ofseparatedStrings
.
– Badhan Ganesh
Dec 13 at 22:05
Could you please point to the SL implementation ofcomponents(separatedBy separator: String) -> [String]
?
– Carpsen90
Dec 16 at 13:15
Right off the bat I can see that the array returned could be named better as
separatedComponents
instead of separatedStrings
.– Badhan Ganesh
Dec 13 at 22:05
Right off the bat I can see that the array returned could be named better as
separatedComponents
instead of separatedStrings
.– Badhan Ganesh
Dec 13 at 22:05
Could you please point to the SL implementation of
components(separatedBy separator: String) -> [String]
?– Carpsen90
Dec 16 at 13:15
Could you please point to the SL implementation of
components(separatedBy separator: String) -> [String]
?– Carpsen90
Dec 16 at 13:15
add a comment |
1 Answer
1
active
oldest
votes
Coding style
This is of course a matter of personal taste, but would split multiple statements like
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
into separate lines
var currentIndex = 0
var stringBuffer = ""
var separatedStrings:[String] =
I would also start new lines for nested code blocks, i.e.
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
becomes
if currentIndex == lastIndex {
separatedStrings.append(stringBuffer)
}
Correctness
Your function takes a separator of type string (protocol) as an argument, but actually works only for separators consisting of a single character. Example:
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: ". "))
// ["Do not be sorry. Be better."]
The reason is that here
if String(character) == separatorString
a single character of the source string is compared with the separator.
Your method also behaves differently from the standard library version when called with an empty string,
"".components(separatedBy: ".")
returns an empty array instead of a single-element array
[""]
. The reason is that the check
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
is never done for an empty input string. (Checking for the last iteration inside a loop always makes me suspicious.)
Simplifications
Instead of converting a character to a string for appending
stringBuffer += .init(character)
you can append it directly:
stringBuffer.append(character)
Keeping track of the current character position can be done with
enumerated()
for (currentIndex, character) in self.enumerated() {
// ...
}
instead of incrementing var currentIndex
.
Efficiency
The main bottleneck is the
var lastIndex:Int
extension method. For Strings (and other collections which are not random accessible) determining self.count
is a O(N) operation (N = number of characters). It requires traversing the entire string.
This method is called for each character in the source string, so that this contributes O(N^2) to the execution time.
It would also be more efficient to locate the next occurrence of the separator and append an entire substring to the result array, instead of appending single characters repeatedly.
Alternative implementation
Here is an alternative implementation, considering the above suggestions:
func components<T>(separatedBy separatorString: T) -> [String]
where T: StringProtocol, Index == String.Index {
var separatedStrings: [String] =
var pos = startIndex
while let range = self[pos...].range(of: separatorString) {
separatedStrings.append(String(self[pos..<range.lowerBound]))
pos = range.upperBound
}
separatedStrings.append(String(self[pos...]))
return separatedStrings
}
@Carpsen90: I doubt it. I assume that the subscript transformspos...
topos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….
– Martin R
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209643%2fimplementing-componentsseparatedby-method-in-swift%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
Coding style
This is of course a matter of personal taste, but would split multiple statements like
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
into separate lines
var currentIndex = 0
var stringBuffer = ""
var separatedStrings:[String] =
I would also start new lines for nested code blocks, i.e.
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
becomes
if currentIndex == lastIndex {
separatedStrings.append(stringBuffer)
}
Correctness
Your function takes a separator of type string (protocol) as an argument, but actually works only for separators consisting of a single character. Example:
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: ". "))
// ["Do not be sorry. Be better."]
The reason is that here
if String(character) == separatorString
a single character of the source string is compared with the separator.
Your method also behaves differently from the standard library version when called with an empty string,
"".components(separatedBy: ".")
returns an empty array instead of a single-element array
[""]
. The reason is that the check
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
is never done for an empty input string. (Checking for the last iteration inside a loop always makes me suspicious.)
Simplifications
Instead of converting a character to a string for appending
stringBuffer += .init(character)
you can append it directly:
stringBuffer.append(character)
Keeping track of the current character position can be done with
enumerated()
for (currentIndex, character) in self.enumerated() {
// ...
}
instead of incrementing var currentIndex
.
Efficiency
The main bottleneck is the
var lastIndex:Int
extension method. For Strings (and other collections which are not random accessible) determining self.count
is a O(N) operation (N = number of characters). It requires traversing the entire string.
This method is called for each character in the source string, so that this contributes O(N^2) to the execution time.
It would also be more efficient to locate the next occurrence of the separator and append an entire substring to the result array, instead of appending single characters repeatedly.
Alternative implementation
Here is an alternative implementation, considering the above suggestions:
func components<T>(separatedBy separatorString: T) -> [String]
where T: StringProtocol, Index == String.Index {
var separatedStrings: [String] =
var pos = startIndex
while let range = self[pos...].range(of: separatorString) {
separatedStrings.append(String(self[pos..<range.lowerBound]))
pos = range.upperBound
}
separatedStrings.append(String(self[pos...]))
return separatedStrings
}
@Carpsen90: I doubt it. I assume that the subscript transformspos...
topos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….
– Martin R
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
add a comment |
Coding style
This is of course a matter of personal taste, but would split multiple statements like
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
into separate lines
var currentIndex = 0
var stringBuffer = ""
var separatedStrings:[String] =
I would also start new lines for nested code blocks, i.e.
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
becomes
if currentIndex == lastIndex {
separatedStrings.append(stringBuffer)
}
Correctness
Your function takes a separator of type string (protocol) as an argument, but actually works only for separators consisting of a single character. Example:
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: ". "))
// ["Do not be sorry. Be better."]
The reason is that here
if String(character) == separatorString
a single character of the source string is compared with the separator.
Your method also behaves differently from the standard library version when called with an empty string,
"".components(separatedBy: ".")
returns an empty array instead of a single-element array
[""]
. The reason is that the check
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
is never done for an empty input string. (Checking for the last iteration inside a loop always makes me suspicious.)
Simplifications
Instead of converting a character to a string for appending
stringBuffer += .init(character)
you can append it directly:
stringBuffer.append(character)
Keeping track of the current character position can be done with
enumerated()
for (currentIndex, character) in self.enumerated() {
// ...
}
instead of incrementing var currentIndex
.
Efficiency
The main bottleneck is the
var lastIndex:Int
extension method. For Strings (and other collections which are not random accessible) determining self.count
is a O(N) operation (N = number of characters). It requires traversing the entire string.
This method is called for each character in the source string, so that this contributes O(N^2) to the execution time.
It would also be more efficient to locate the next occurrence of the separator and append an entire substring to the result array, instead of appending single characters repeatedly.
Alternative implementation
Here is an alternative implementation, considering the above suggestions:
func components<T>(separatedBy separatorString: T) -> [String]
where T: StringProtocol, Index == String.Index {
var separatedStrings: [String] =
var pos = startIndex
while let range = self[pos...].range(of: separatorString) {
separatedStrings.append(String(self[pos..<range.lowerBound]))
pos = range.upperBound
}
separatedStrings.append(String(self[pos...]))
return separatedStrings
}
@Carpsen90: I doubt it. I assume that the subscript transformspos...
topos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….
– Martin R
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
add a comment |
Coding style
This is of course a matter of personal taste, but would split multiple statements like
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
into separate lines
var currentIndex = 0
var stringBuffer = ""
var separatedStrings:[String] =
I would also start new lines for nested code blocks, i.e.
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
becomes
if currentIndex == lastIndex {
separatedStrings.append(stringBuffer)
}
Correctness
Your function takes a separator of type string (protocol) as an argument, but actually works only for separators consisting of a single character. Example:
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: ". "))
// ["Do not be sorry. Be better."]
The reason is that here
if String(character) == separatorString
a single character of the source string is compared with the separator.
Your method also behaves differently from the standard library version when called with an empty string,
"".components(separatedBy: ".")
returns an empty array instead of a single-element array
[""]
. The reason is that the check
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
is never done for an empty input string. (Checking for the last iteration inside a loop always makes me suspicious.)
Simplifications
Instead of converting a character to a string for appending
stringBuffer += .init(character)
you can append it directly:
stringBuffer.append(character)
Keeping track of the current character position can be done with
enumerated()
for (currentIndex, character) in self.enumerated() {
// ...
}
instead of incrementing var currentIndex
.
Efficiency
The main bottleneck is the
var lastIndex:Int
extension method. For Strings (and other collections which are not random accessible) determining self.count
is a O(N) operation (N = number of characters). It requires traversing the entire string.
This method is called for each character in the source string, so that this contributes O(N^2) to the execution time.
It would also be more efficient to locate the next occurrence of the separator and append an entire substring to the result array, instead of appending single characters repeatedly.
Alternative implementation
Here is an alternative implementation, considering the above suggestions:
func components<T>(separatedBy separatorString: T) -> [String]
where T: StringProtocol, Index == String.Index {
var separatedStrings: [String] =
var pos = startIndex
while let range = self[pos...].range(of: separatorString) {
separatedStrings.append(String(self[pos..<range.lowerBound]))
pos = range.upperBound
}
separatedStrings.append(String(self[pos...]))
return separatedStrings
}
Coding style
This is of course a matter of personal taste, but would split multiple statements like
var currentIndex = 0; var stringBuffer = ""; var separatedStrings:[String] =
into separate lines
var currentIndex = 0
var stringBuffer = ""
var separatedStrings:[String] =
I would also start new lines for nested code blocks, i.e.
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
becomes
if currentIndex == lastIndex {
separatedStrings.append(stringBuffer)
}
Correctness
Your function takes a separator of type string (protocol) as an argument, but actually works only for separators consisting of a single character. Example:
let sampleString = "Do not be sorry. Be better."
print(sampleString.components(separatedBy: ". "))
// ["Do not be sorry. Be better."]
The reason is that here
if String(character) == separatorString
a single character of the source string is compared with the separator.
Your method also behaves differently from the standard library version when called with an empty string,
"".components(separatedBy: ".")
returns an empty array instead of a single-element array
[""]
. The reason is that the check
if currentIndex == lastIndex { separatedStrings.append(stringBuffer) }
is never done for an empty input string. (Checking for the last iteration inside a loop always makes me suspicious.)
Simplifications
Instead of converting a character to a string for appending
stringBuffer += .init(character)
you can append it directly:
stringBuffer.append(character)
Keeping track of the current character position can be done with
enumerated()
for (currentIndex, character) in self.enumerated() {
// ...
}
instead of incrementing var currentIndex
.
Efficiency
The main bottleneck is the
var lastIndex:Int
extension method. For Strings (and other collections which are not random accessible) determining self.count
is a O(N) operation (N = number of characters). It requires traversing the entire string.
This method is called for each character in the source string, so that this contributes O(N^2) to the execution time.
It would also be more efficient to locate the next occurrence of the separator and append an entire substring to the result array, instead of appending single characters repeatedly.
Alternative implementation
Here is an alternative implementation, considering the above suggestions:
func components<T>(separatedBy separatorString: T) -> [String]
where T: StringProtocol, Index == String.Index {
var separatedStrings: [String] =
var pos = startIndex
while let range = self[pos...].range(of: separatorString) {
separatedStrings.append(String(self[pos..<range.lowerBound]))
pos = range.upperBound
}
separatedStrings.append(String(self[pos...]))
return separatedStrings
}
edited Dec 13 at 23:31
answered Dec 13 at 22:46
Martin R
15.5k12264
15.5k12264
@Carpsen90: I doubt it. I assume that the subscript transformspos...
topos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….
– Martin R
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
add a comment |
@Carpsen90: I doubt it. I assume that the subscript transformspos...
topos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….
– Martin R
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
@Carpsen90: I doubt it. I assume that the subscript transforms
pos...
to pos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….– Martin R
yesterday
@Carpsen90: I doubt it. I assume that the subscript transforms
pos...
to pos..<endIndex
using this method, so there should be no difference. – See also github.com/apple/swift-evolution/blob/master/proposals/….– Martin R
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
Thanks. I had a faint doubt that that transformation would cost some extra work, either at compile or run times.
– Carpsen90
yesterday
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f209643%2fimplementing-componentsseparatedby-method-in-swift%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Right off the bat I can see that the array returned could be named better as
separatedComponents
instead ofseparatedStrings
.– Badhan Ganesh
Dec 13 at 22:05
Could you please point to the SL implementation of
components(separatedBy separator: String) -> [String]
?– Carpsen90
Dec 16 at 13:15