Added 'splice' function to the string object#2642
Added 'splice' function to the string object#2642AmatanHead wants to merge 3 commits intomootools:masterfrom AmatanHead:string.splice
Conversation
|
Your pull request looks really good! Also there is no doubt this function can be useful. The question is whether this should be in -core, or somewhere else. Let's hear what others have to say... |
There was a problem hiding this comment.
This doesn't seem intuitive. One could argue this would have been the result: 'Add substring'
There was a problem hiding this comment.
I know it's suppose to mimic Array.splice ... just not exactly very readable.
|
If this is useful for anyone else, then we can go ahead and pull into Core. I'm arguing the readability, but it's a moot argument if others pick up the idea. |
|
|
There was a problem hiding this comment.
If it should work the same as Array#splice, you should be able to insert all rest arguments in between.
|
String#splice should obviously work the same as Array#splice. Too bad |
|
One difference, looking at Array#splice, is that it returns an array of the removed items. |
|
I see two ways to fix this: first is to try to fully mimic array.splice, second is to brake intuitive similarity and invite something new. |
There was a problem hiding this comment.
What if remove was < 0?
What if remove and insert was missing?
What if position, remove, and insert were missing?
There was a problem hiding this comment.
What if remove was > this.length?
There was a problem hiding this comment.
What if position was > this.length? Pad with empty string?
|
You can also just delegate to splice? var t = this.split("");
t.splice(pos, rem, ins);
return t.join(""); |
|
Re: the stick to Array.splice mimicry vs. venture into splice-like I would start with the Specs, and define the questions and answers. I'd personally prefer to write something like: 'Some string'.splice('my other string', 5); // 'Some my other string'
'abc'.splice(' ', 1); // 'a c'
'abc'.splice(' ', 1, 2); // 'a c'
myString.splice(withAnother, optStartPosition, optLength);
'abc'.splice('def'); // 'abcdef' |
|
@ibolmo what if you just want to remove a part of the string, and don't want to insert something new, then rather than passing an empty string/value (null/undefined), the replacement string should be the last argument, like it is in Array.splice... Then it's really optional. |
|
If so, I would prefer to split this operation into paste(what, optPosition), insert(what, optPosition) and delete(length, optPosition). [bad idea] |
|
... or, as suggests @megawac, just delegate it. May even better: or return two arguments: |
|
So here's a new implementation. Now it's really close to array.splice because it's based on array.splice.
|
|
Is something wrong to this PR? Why so silent here? |
|
@AmatanHead really nice PR! @ibolmo, @arian anything you would like to comment, discuss or add here? |
|
splice is a modificative function for arrays. having splice on strings would assume it either is. But strings cannot be modified in JavaScript, you still get a new string. If you really need this method, IMO, it should be slice, not splice. |
|
I share the opinion of @nfroidure. Consider the following code: String.implement({
splice: function() {
arguments[0] += (arguments[0] < 0 ? this.length + 1 : 0);
var return_both = (typeof arguments[arguments.length - 1] === 'boolean' ? arguments[--arguments.length] : false),
t = this.split(""),
change = t.splice.apply(t, arguments).join(""),
out = t.join("");
return (return_both ? [out, change] : out);
}
})
var str = "Madonna"
, arr = [ 1, 2, 3, 4 ]
str.splice(2, 1)
arr.splice(2, 1)
console.log(str)
console.log(arr)
The function should be renamed as https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/slice |
A splice function for strings could be useful. It takes string and inserts another string before the specified position.