Implementing `components(separatedBy:)` method in Swift












1














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
}
}
}









share|improve this question






















  • 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
















1














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
}
}
}









share|improve this question






















  • 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














1












1








1







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
}
}
}









share|improve this question













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






share|improve this question













share|improve this question











share|improve this question




share|improve this question










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 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


















  • 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
















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










1 Answer
1






active

oldest

votes


















1














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
}





share|improve this answer























  • @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











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
});


}
});














draft saved

draft discarded


















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









1














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
}





share|improve this answer























  • @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
















1














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
}





share|improve this answer























  • @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














1












1








1






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
}





share|improve this answer














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
}






share|improve this answer














share|improve this answer



share|improve this answer








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 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


















  • @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
















@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


















draft saved

draft discarded




















































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.




draft saved


draft discarded














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





















































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







Popular posts from this blog

Сан-Квентин

8-я гвардейская общевойсковая армия

Алькесар