Cropping a message using array splitsMilliseconds to Time string & Time string to MillisecondsVarious simple JavaScript solutionsDefinitional Returns. Solved. MostlyBaking homemade pies has made me so many new connectionsA String.prototype.diff() implementation (text diff)Simple Java password rule enforcingSmall JavaScript library for ECMAScript version detectionThis macro will convert you… or notApproximate string search in PythonImporting text into PANDAS and counting certain words
Are runways booked by airlines to land their planes?
Possibility of faking someone's public key
Is there an idiom that means that you are in a very strong negotiation position in a negotiation?
Why do Russians almost not use verbs of possession akin to "have"?
Can you still travel to America on the ESTA waiver program if you have been to Iran in transit?
Dad jokes are fun
A burglar's sunglasses, a lady's odyssey
The Maltese Falcon
First Program Tic-Tac-Toe
Can we assume that a hash function with high collision resistance also means highly uniform distribution?
Security vulnerabilities of POST over SSL
Who knighted this Game of Thrones character?
Why was this character made Grand Maester?
How to determine if a hyphen (-) exists inside a column
One word for 'the thing that attracts me'?
Why is unzipped directory exactly 4.0k (much smaller than zipped file)?
Why would a rational buyer offer to buy with no conditions precedent?
Why does Bran want to find Drogon?
Navigating a quick return to previous employer
What did the 'turbo' button actually do?
Why did Jon Snow do this immoral act if he is so honorable?
How can I properly write this equation in Latex?
Can a ring of spell storing and access to Find spells produce an endless menagerie?
Creating second map without labels using QGIS?
Cropping a message using array splits
Milliseconds to Time string & Time string to MillisecondsVarious simple JavaScript solutionsDefinitional Returns. Solved. MostlyBaking homemade pies has made me so many new connectionsA String.prototype.diff() implementation (text diff)Simple Java password rule enforcingSmall JavaScript library for ECMAScript version detectionThis macro will convert you… or notApproximate string search in PythonImporting text into PANDAS and counting certain words
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
I am trying to write a simple function to crop a given message to a specific length but at the same time not to cut the words in between and no trailing spaces in the end.
Example:
Input String: The quick brown fox jumped over the fence, K: 11
Output: The quick
Here is what I have tried:
function crop(message, K)
var originalLen = message.length;
if(originalLen<K)
return message;
else
var words = message.split(' '),substr;
for(var i=words.length;i > 0;i--)
words.pop();
if(words.join(' ').length<=K)
return words.join(' ');
This function works fine but I am not very happy with the implementation. Need suggestions on the performance aspects and will there be a case where this won't work?
javascript performance strings array formatting
$endgroup$
add a comment |
$begingroup$
I am trying to write a simple function to crop a given message to a specific length but at the same time not to cut the words in between and no trailing spaces in the end.
Example:
Input String: The quick brown fox jumped over the fence, K: 11
Output: The quick
Here is what I have tried:
function crop(message, K)
var originalLen = message.length;
if(originalLen<K)
return message;
else
var words = message.split(' '),substr;
for(var i=words.length;i > 0;i--)
words.pop();
if(words.join(' ').length<=K)
return words.join(' ');
This function works fine but I am not very happy with the implementation. Need suggestions on the performance aspects and will there be a case where this won't work?
javascript performance strings array formatting
$endgroup$
add a comment |
$begingroup$
I am trying to write a simple function to crop a given message to a specific length but at the same time not to cut the words in between and no trailing spaces in the end.
Example:
Input String: The quick brown fox jumped over the fence, K: 11
Output: The quick
Here is what I have tried:
function crop(message, K)
var originalLen = message.length;
if(originalLen<K)
return message;
else
var words = message.split(' '),substr;
for(var i=words.length;i > 0;i--)
words.pop();
if(words.join(' ').length<=K)
return words.join(' ');
This function works fine but I am not very happy with the implementation. Need suggestions on the performance aspects and will there be a case where this won't work?
javascript performance strings array formatting
$endgroup$
I am trying to write a simple function to crop a given message to a specific length but at the same time not to cut the words in between and no trailing spaces in the end.
Example:
Input String: The quick brown fox jumped over the fence, K: 11
Output: The quick
Here is what I have tried:
function crop(message, K)
var originalLen = message.length;
if(originalLen<K)
return message;
else
var words = message.split(' '),substr;
for(var i=words.length;i > 0;i--)
words.pop();
if(words.join(' ').length<=K)
return words.join(' ');
This function works fine but I am not very happy with the implementation. Need suggestions on the performance aspects and will there be a case where this won't work?
javascript performance strings array formatting
javascript performance strings array formatting
edited May 10 at 3:32
200_success
132k20159426
132k20159426
asked May 9 at 20:29
beNerdbeNerd
1674
1674
add a comment |
add a comment |
4 Answers
4
active
oldest
votes
$begingroup$
This is much slower than necessary. It takes time to construct the array, and more to shorten the array word-by-word. It's easy to imagine how this would go if words
contains a whole book and K
is some small number.
In general, you want an approach that inspects the original string to decide how much to keep, and then extracts that much, once, before returning it.
A regular expression is an efficient and compact way to find text that meets your criteria. Consider:
function crop(message, K)
if(K<1) return "";
const reK = new RegExp( `^.0,$K-1[^ ](?=
.match
returns an array with the matched text as the first element, or null
if no match. The alternative [ "" ]
will provide an empty string as a return value if there is no match (when the first word is longer than K
).
The regular expression, broken down, means:
^
: match start of string.
: followed by any character0,10
: ... up to ten times (one less thanK
)[^ ]
: followed by a character that is not a space(?=…)
: this is an assertion; it means the following expression must match, but is not included in the result:: followed by a space
|
: or$
: end-of-string
Exercise: can you generalize this approach to recognize any kind of whitespace (tabs, newlines, and so on)?
$endgroup$
$begingroup$
Why do you have[^ ]
? When thinking of what the answer would be I got/^.1,11(?=s)/
yours is better as it check for$
too. But I can't understand the addition of[^ ]
.
$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.
$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
add a comment |
$begingroup$
A fairly simple alternative. Take the maxLength
String plus one letter and cut it at the last space. If the maxLength
was at the end of a word, the "plus one letter" will take care of that.
The > signs in the tests are there to make any trailing spaces visible.
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
The other answers have very good explanations. I just felt a really simple solution was missing.
$endgroup$
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
add a comment |
$begingroup$
Your code looks great.
Oh My Goodness's solution is really great.
If you wish, you might be able to design an expression that would do the entire process. I'm not so sure about my expression in this link, but it might give you an idea, how you may do so:
([A-z0-9s]1,11)(s)(.*)
This expression is relaxed from the right and has three capturing groups with just a list of chars that I have just added in the first capturing group and I'm sure you might want to change that list.
You may also want to add or reduce the boundaries.
Graph
This graph shows how the expression would work and you can visualize other expressions in this link:
Performance Test
This JavaScript snippet shows the performance of that expression using a simple 1-million times for
loop.
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
Testing Code
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
$endgroup$
1
$begingroup$
FWIW I found(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.
$endgroup$
– Peilonrayz
May 10 at 1:59
1
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
add a comment |
$begingroup$
A Code Review
Your code is a mess,
- Inconsistent indenting.
- Poor use of space between tokens, and operators.
- Inappropriate use of variable declaration type
let
,var
,const
. - Contains irrelevant / unused code. eg
substr
Fails to meet requirements.
You list the requirement
"no trailing spaces in the end."
Yet your code fails to do this in two ways
When string is shorter than required length
crop("trailing spaces ", 100); // returns "trailing spaces "
When string contains 2 or more spaces near required length.
crop("Trailing spaces strings with extra spaces", 17); // returns "Trailing spaces "
Note: There are various white space characters not just the space. There are also special unicode characters the are visually 1 character (depending on device OS) yet take up 2 or more characters. eg "👨🚀".length === 5
is true
. All JavaScript strings are Unicode.
Rewrite
Using the same logic (build return string from array of split words) the following example attempts to correct the style and adherence to the requirements.
I prefer 4 space indentation (using spaces not tabs as tabs always seem to stuff up when copying between systems) however 2 spaces is acceptable (only by popularity)
I assume that the message
was converted from ASCII and spaces are the only white spaces of concern.
function crop(message, maxLength) { // use meaningful names
var result = message.trimEnd(); // Use var for function scoped variable
if (result.length > maxLength) // space between if ( > and )
const words = result.split(" "); // use const for variables that do not change
do
words.pop();
result = words.join(" ").trimEnd(); // ensure no trailing spaces
if (result.length <= maxLength) // not repeating same join operation
break;
while (words.length);
return result;
Note: Check runtime has String.trimEnd
or use a polyfill or transpiler.
Update $O(1)$ solution
I forgot to put in a better solution.
Rebuilding the string is slow, or passing the string through a regExp requires iteration over the whole string.
By looking at the character at the desired length you can workout if you need to move down to find the next space and then return the end trimmed sub string result, or just return the end Trimmed sub string.
The result has a complexity of $O(1)$ or in terms of $n = K$ (maxLength) $O(n)$
function crop(message, maxLength)
if (maxLength < 1) return ""
if (message.length <= maxLength) return message.trimEnd()
maxLength++;
while (--maxLength && message[maxLength] !== " ");
return message.substring(0, maxLength).trimEnd();
It is significantly faster than any other solutions in this question.
$endgroup$
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
add a comment |
Your Answer
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%2f220011%2fcropping-a-message-using-array-splits%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
4 Answers
4
active
oldest
votes
4 Answers
4
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
This is much slower than necessary. It takes time to construct the array, and more to shorten the array word-by-word. It's easy to imagine how this would go if words
contains a whole book and K
is some small number.
In general, you want an approach that inspects the original string to decide how much to keep, and then extracts that much, once, before returning it.
A regular expression is an efficient and compact way to find text that meets your criteria. Consider:
function crop(message, K)
if(K<1) return "";
const reK = new RegExp( `^.0,$K-1[^ ](?=
.match
returns an array with the matched text as the first element, or null
if no match. The alternative [ "" ]
will provide an empty string as a return value if there is no match (when the first word is longer than K
).
The regular expression, broken down, means:
^
: match start of string.
: followed by any character0,10
: ... up to ten times (one less thanK
)[^ ]
: followed by a character that is not a space(?=…)
: this is an assertion; it means the following expression must match, but is not included in the result:: followed by a space
|
: or$
: end-of-string
Exercise: can you generalize this approach to recognize any kind of whitespace (tabs, newlines, and so on)?
$endgroup$
$begingroup$
Why do you have[^ ]
? When thinking of what the answer would be I got/^.1,11(?=s)/
yours is better as it check for$
too. But I can't understand the addition of[^ ]
.
$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.
$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
add a comment |
$begingroup$
This is much slower than necessary. It takes time to construct the array, and more to shorten the array word-by-word. It's easy to imagine how this would go if words
contains a whole book and K
is some small number.
In general, you want an approach that inspects the original string to decide how much to keep, and then extracts that much, once, before returning it.
A regular expression is an efficient and compact way to find text that meets your criteria. Consider:
function crop(message, K)
if(K<1) return "";
const reK = new RegExp( `^.0,$K-1[^ ](?=
.match
returns an array with the matched text as the first element, or null
if no match. The alternative [ "" ]
will provide an empty string as a return value if there is no match (when the first word is longer than K
).
The regular expression, broken down, means:
^
: match start of string.
: followed by any character0,10
: ... up to ten times (one less thanK
)[^ ]
: followed by a character that is not a space(?=…)
: this is an assertion; it means the following expression must match, but is not included in the result:: followed by a space
|
: or$
: end-of-string
Exercise: can you generalize this approach to recognize any kind of whitespace (tabs, newlines, and so on)?
$endgroup$
$begingroup$
Why do you have[^ ]
? When thinking of what the answer would be I got/^.1,11(?=s)/
yours is better as it check for$
too. But I can't understand the addition of[^ ]
.
$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.
$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
add a comment |
$begingroup$
This is much slower than necessary. It takes time to construct the array, and more to shorten the array word-by-word. It's easy to imagine how this would go if words
contains a whole book and K
is some small number.
In general, you want an approach that inspects the original string to decide how much to keep, and then extracts that much, once, before returning it.
A regular expression is an efficient and compact way to find text that meets your criteria. Consider:
function crop(message, K)
if(K<1) return "";
const reK = new RegExp( `^.0,$K-1[^ ](?=
.match
returns an array with the matched text as the first element, or null
if no match. The alternative [ "" ]
will provide an empty string as a return value if there is no match (when the first word is longer than K
).
The regular expression, broken down, means:
^
: match start of string.
: followed by any character0,10
: ... up to ten times (one less thanK
)[^ ]
: followed by a character that is not a space(?=…)
: this is an assertion; it means the following expression must match, but is not included in the result:: followed by a space
|
: or$
: end-of-string
Exercise: can you generalize this approach to recognize any kind of whitespace (tabs, newlines, and so on)?
$endgroup$
This is much slower than necessary. It takes time to construct the array, and more to shorten the array word-by-word. It's easy to imagine how this would go if words
contains a whole book and K
is some small number.
In general, you want an approach that inspects the original string to decide how much to keep, and then extracts that much, once, before returning it.
A regular expression is an efficient and compact way to find text that meets your criteria. Consider:
function crop(message, K)
if(K<1) return "";
const reK = new RegExp( `^.0,$K-1[^ ](?=
.match
returns an array with the matched text as the first element, or null
if no match. The alternative [ "" ]
will provide an empty string as a return value if there is no match (when the first word is longer than K
).
The regular expression, broken down, means:
^
: match start of string.
: followed by any character0,10
: ... up to ten times (one less thanK
)[^ ]
: followed by a character that is not a space(?=…)
: this is an assertion; it means the following expression must match, but is not included in the result:: followed by a space
|
: or$
: end-of-string
Exercise: can you generalize this approach to recognize any kind of whitespace (tabs, newlines, and so on)?
answered May 9 at 21:16
Oh My GoodnessOh My Goodness
2,826316
2,826316
$begingroup$
Why do you have[^ ]
? When thinking of what the answer would be I got/^.1,11(?=s)/
yours is better as it check for$
too. But I can't understand the addition of[^ ]
.
$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.
$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
add a comment |
$begingroup$
Why do you have[^ ]
? When thinking of what the answer would be I got/^.1,11(?=s)/
yours is better as it check for$
too. But I can't understand the addition of[^ ]
.
$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.
$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
$begingroup$
Why do you have
[^ ]
? When thinking of what the answer would be I got /^.1,11(?=s)/
yours is better as it check for $
too. But I can't understand the addition of [^ ]
.$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
Why do you have
[^ ]
? When thinking of what the answer would be I got /^.1,11(?=s)/
yours is better as it check for $
too. But I can't understand the addition of [^ ]
.$endgroup$
– Peilonrayz
May 10 at 2:07
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
/^.1,11(?=s)/
will include a trailing space in the match if there are two spaces together.$endgroup$
– Oh My Goodness
May 10 at 2:41
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
$begingroup$
Ah, that makes sense. Thank you :)
$endgroup$
– Peilonrayz
May 10 at 2:44
add a comment |
$begingroup$
A fairly simple alternative. Take the maxLength
String plus one letter and cut it at the last space. If the maxLength
was at the end of a word, the "plus one letter" will take care of that.
The > signs in the tests are there to make any trailing spaces visible.
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
The other answers have very good explanations. I just felt a really simple solution was missing.
$endgroup$
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
add a comment |
$begingroup$
A fairly simple alternative. Take the maxLength
String plus one letter and cut it at the last space. If the maxLength
was at the end of a word, the "plus one letter" will take care of that.
The > signs in the tests are there to make any trailing spaces visible.
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
The other answers have very good explanations. I just felt a really simple solution was missing.
$endgroup$
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
add a comment |
$begingroup$
A fairly simple alternative. Take the maxLength
String plus one letter and cut it at the last space. If the maxLength
was at the end of a word, the "plus one letter" will take care of that.
The > signs in the tests are there to make any trailing spaces visible.
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
The other answers have very good explanations. I just felt a really simple solution was missing.
$endgroup$
A fairly simple alternative. Take the maxLength
String plus one letter and cut it at the last space. If the maxLength
was at the end of a word, the "plus one letter" will take care of that.
The > signs in the tests are there to make any trailing spaces visible.
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
The other answers have very good explanations. I just felt a really simple solution was missing.
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
const crop = (message, maxLength) =>
const part = message.substring(0, maxLength + 1);
return part.substring(0, part.lastIndexOf(" ")).trimEnd();
console.log(crop("The quick brown fox jumped over the fence", 11)+">");
console.log(crop("The quick brown fox jumped over the fence", 9)+">");
console.log(crop("The quick brown fox jumped over the fence", 8)+">");
console.log(crop("The ", 6)+">");
console.log(crop("The quick ", 20)+">");
edited May 10 at 10:57
answered May 10 at 10:52
JollyJokerJollyJoker
42126
42126
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
add a comment |
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
$begingroup$
+1. I think this is actually a really elegant way to accomplish the request.
$endgroup$
– KGlasier
May 10 at 15:45
add a comment |
$begingroup$
Your code looks great.
Oh My Goodness's solution is really great.
If you wish, you might be able to design an expression that would do the entire process. I'm not so sure about my expression in this link, but it might give you an idea, how you may do so:
([A-z0-9s]1,11)(s)(.*)
This expression is relaxed from the right and has three capturing groups with just a list of chars that I have just added in the first capturing group and I'm sure you might want to change that list.
You may also want to add or reduce the boundaries.
Graph
This graph shows how the expression would work and you can visualize other expressions in this link:
Performance Test
This JavaScript snippet shows the performance of that expression using a simple 1-million times for
loop.
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
Testing Code
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
$endgroup$
1
$begingroup$
FWIW I found(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.
$endgroup$
– Peilonrayz
May 10 at 1:59
1
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
add a comment |
$begingroup$
Your code looks great.
Oh My Goodness's solution is really great.
If you wish, you might be able to design an expression that would do the entire process. I'm not so sure about my expression in this link, but it might give you an idea, how you may do so:
([A-z0-9s]1,11)(s)(.*)
This expression is relaxed from the right and has three capturing groups with just a list of chars that I have just added in the first capturing group and I'm sure you might want to change that list.
You may also want to add or reduce the boundaries.
Graph
This graph shows how the expression would work and you can visualize other expressions in this link:
Performance Test
This JavaScript snippet shows the performance of that expression using a simple 1-million times for
loop.
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
Testing Code
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
$endgroup$
1
$begingroup$
FWIW I found(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.
$endgroup$
– Peilonrayz
May 10 at 1:59
1
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
add a comment |
$begingroup$
Your code looks great.
Oh My Goodness's solution is really great.
If you wish, you might be able to design an expression that would do the entire process. I'm not so sure about my expression in this link, but it might give you an idea, how you may do so:
([A-z0-9s]1,11)(s)(.*)
This expression is relaxed from the right and has three capturing groups with just a list of chars that I have just added in the first capturing group and I'm sure you might want to change that list.
You may also want to add or reduce the boundaries.
Graph
This graph shows how the expression would work and you can visualize other expressions in this link:
Performance Test
This JavaScript snippet shows the performance of that expression using a simple 1-million times for
loop.
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
Testing Code
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
$endgroup$
Your code looks great.
Oh My Goodness's solution is really great.
If you wish, you might be able to design an expression that would do the entire process. I'm not so sure about my expression in this link, but it might give you an idea, how you may do so:
([A-z0-9s]1,11)(s)(.*)
This expression is relaxed from the right and has three capturing groups with just a list of chars that I have just added in the first capturing group and I'm sure you might want to change that list.
You may also want to add or reduce the boundaries.
Graph
This graph shows how the expression would work and you can visualize other expressions in this link:
Performance Test
This JavaScript snippet shows the performance of that expression using a simple 1-million times for
loop.
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
Testing Code
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
const repeat = 1000000;
const start = Date.now();
for (var i = repeat; i >= 0; i--)
const string = 'The quick brown fox jumped over the fence';
const regex = /([A-z0-9s]1,11)(s)(.*)/gm;
var match = string.replace(regex, "$1");
const end = Date.now() - start;
console.log("YAAAY! "" + match + "" is a match 💚💚💚 ");
console.log(end / 1000 + " is the runtime of " + repeat + " times benchmark test. 😳 ");
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
const regex = /([A-z0-9s]1,11)(s)(.*)/s;
const str = `The quick brown fox jumped over the fence`;
const subst = `$1`;
// The substituted value will be contained in the result variable
const result = str.replace(regex, subst);
console.log('Substitution result: ', result);
edited May 9 at 22:55
answered May 9 at 22:49
EmmaEmma
2701215
2701215
1
$begingroup$
FWIW I found(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.
$endgroup$
– Peilonrayz
May 10 at 1:59
1
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
add a comment |
1
$begingroup$
FWIW I found(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.
$endgroup$
– Peilonrayz
May 10 at 1:59
1
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
1
1
$begingroup$
FWIW I found
(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.$endgroup$
– Peilonrayz
May 10 at 1:59
$begingroup$
FWIW I found
(/^.1,11(?=s)/gm).exec(string)[0]
to be much faster. (FF) I also think it's simpler, but given there's a forward reference I can see why others might not agree.$endgroup$
– Peilonrayz
May 10 at 1:59
1
1
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
$begingroup$
I saw a performance test and the urge to try it out over came me, looking at the other answer it's a slightly worse version than Oh My Goodness'. Nice answer btw :)
$endgroup$
– Peilonrayz
May 10 at 2:10
add a comment |
$begingroup$
A Code Review
Your code is a mess,
- Inconsistent indenting.
- Poor use of space between tokens, and operators.
- Inappropriate use of variable declaration type
let
,var
,const
. - Contains irrelevant / unused code. eg
substr
Fails to meet requirements.
You list the requirement
"no trailing spaces in the end."
Yet your code fails to do this in two ways
When string is shorter than required length
crop("trailing spaces ", 100); // returns "trailing spaces "
When string contains 2 or more spaces near required length.
crop("Trailing spaces strings with extra spaces", 17); // returns "Trailing spaces "
Note: There are various white space characters not just the space. There are also special unicode characters the are visually 1 character (depending on device OS) yet take up 2 or more characters. eg "👨🚀".length === 5
is true
. All JavaScript strings are Unicode.
Rewrite
Using the same logic (build return string from array of split words) the following example attempts to correct the style and adherence to the requirements.
I prefer 4 space indentation (using spaces not tabs as tabs always seem to stuff up when copying between systems) however 2 spaces is acceptable (only by popularity)
I assume that the message
was converted from ASCII and spaces are the only white spaces of concern.
function crop(message, maxLength) { // use meaningful names
var result = message.trimEnd(); // Use var for function scoped variable
if (result.length > maxLength) // space between if ( > and )
const words = result.split(" "); // use const for variables that do not change
do
words.pop();
result = words.join(" ").trimEnd(); // ensure no trailing spaces
if (result.length <= maxLength) // not repeating same join operation
break;
while (words.length);
return result;
Note: Check runtime has String.trimEnd
or use a polyfill or transpiler.
Update $O(1)$ solution
I forgot to put in a better solution.
Rebuilding the string is slow, or passing the string through a regExp requires iteration over the whole string.
By looking at the character at the desired length you can workout if you need to move down to find the next space and then return the end trimmed sub string result, or just return the end Trimmed sub string.
The result has a complexity of $O(1)$ or in terms of $n = K$ (maxLength) $O(n)$
function crop(message, maxLength)
if (maxLength < 1) return ""
if (message.length <= maxLength) return message.trimEnd()
maxLength++;
while (--maxLength && message[maxLength] !== " ");
return message.substring(0, maxLength).trimEnd();
It is significantly faster than any other solutions in this question.
$endgroup$
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
add a comment |
$begingroup$
A Code Review
Your code is a mess,
- Inconsistent indenting.
- Poor use of space between tokens, and operators.
- Inappropriate use of variable declaration type
let
,var
,const
. - Contains irrelevant / unused code. eg
substr
Fails to meet requirements.
You list the requirement
"no trailing spaces in the end."
Yet your code fails to do this in two ways
When string is shorter than required length
crop("trailing spaces ", 100); // returns "trailing spaces "
When string contains 2 or more spaces near required length.
crop("Trailing spaces strings with extra spaces", 17); // returns "Trailing spaces "
Note: There are various white space characters not just the space. There are also special unicode characters the are visually 1 character (depending on device OS) yet take up 2 or more characters. eg "👨🚀".length === 5
is true
. All JavaScript strings are Unicode.
Rewrite
Using the same logic (build return string from array of split words) the following example attempts to correct the style and adherence to the requirements.
I prefer 4 space indentation (using spaces not tabs as tabs always seem to stuff up when copying between systems) however 2 spaces is acceptable (only by popularity)
I assume that the message
was converted from ASCII and spaces are the only white spaces of concern.
function crop(message, maxLength) { // use meaningful names
var result = message.trimEnd(); // Use var for function scoped variable
if (result.length > maxLength) // space between if ( > and )
const words = result.split(" "); // use const for variables that do not change
do
words.pop();
result = words.join(" ").trimEnd(); // ensure no trailing spaces
if (result.length <= maxLength) // not repeating same join operation
break;
while (words.length);
return result;
Note: Check runtime has String.trimEnd
or use a polyfill or transpiler.
Update $O(1)$ solution
I forgot to put in a better solution.
Rebuilding the string is slow, or passing the string through a regExp requires iteration over the whole string.
By looking at the character at the desired length you can workout if you need to move down to find the next space and then return the end trimmed sub string result, or just return the end Trimmed sub string.
The result has a complexity of $O(1)$ or in terms of $n = K$ (maxLength) $O(n)$
function crop(message, maxLength)
if (maxLength < 1) return ""
if (message.length <= maxLength) return message.trimEnd()
maxLength++;
while (--maxLength && message[maxLength] !== " ");
return message.substring(0, maxLength).trimEnd();
It is significantly faster than any other solutions in this question.
$endgroup$
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
add a comment |
$begingroup$
A Code Review
Your code is a mess,
- Inconsistent indenting.
- Poor use of space between tokens, and operators.
- Inappropriate use of variable declaration type
let
,var
,const
. - Contains irrelevant / unused code. eg
substr
Fails to meet requirements.
You list the requirement
"no trailing spaces in the end."
Yet your code fails to do this in two ways
When string is shorter than required length
crop("trailing spaces ", 100); // returns "trailing spaces "
When string contains 2 or more spaces near required length.
crop("Trailing spaces strings with extra spaces", 17); // returns "Trailing spaces "
Note: There are various white space characters not just the space. There are also special unicode characters the are visually 1 character (depending on device OS) yet take up 2 or more characters. eg "👨🚀".length === 5
is true
. All JavaScript strings are Unicode.
Rewrite
Using the same logic (build return string from array of split words) the following example attempts to correct the style and adherence to the requirements.
I prefer 4 space indentation (using spaces not tabs as tabs always seem to stuff up when copying between systems) however 2 spaces is acceptable (only by popularity)
I assume that the message
was converted from ASCII and spaces are the only white spaces of concern.
function crop(message, maxLength) { // use meaningful names
var result = message.trimEnd(); // Use var for function scoped variable
if (result.length > maxLength) // space between if ( > and )
const words = result.split(" "); // use const for variables that do not change
do
words.pop();
result = words.join(" ").trimEnd(); // ensure no trailing spaces
if (result.length <= maxLength) // not repeating same join operation
break;
while (words.length);
return result;
Note: Check runtime has String.trimEnd
or use a polyfill or transpiler.
Update $O(1)$ solution
I forgot to put in a better solution.
Rebuilding the string is slow, or passing the string through a regExp requires iteration over the whole string.
By looking at the character at the desired length you can workout if you need to move down to find the next space and then return the end trimmed sub string result, or just return the end Trimmed sub string.
The result has a complexity of $O(1)$ or in terms of $n = K$ (maxLength) $O(n)$
function crop(message, maxLength)
if (maxLength < 1) return ""
if (message.length <= maxLength) return message.trimEnd()
maxLength++;
while (--maxLength && message[maxLength] !== " ");
return message.substring(0, maxLength).trimEnd();
It is significantly faster than any other solutions in this question.
$endgroup$
A Code Review
Your code is a mess,
- Inconsistent indenting.
- Poor use of space between tokens, and operators.
- Inappropriate use of variable declaration type
let
,var
,const
. - Contains irrelevant / unused code. eg
substr
Fails to meet requirements.
You list the requirement
"no trailing spaces in the end."
Yet your code fails to do this in two ways
When string is shorter than required length
crop("trailing spaces ", 100); // returns "trailing spaces "
When string contains 2 or more spaces near required length.
crop("Trailing spaces strings with extra spaces", 17); // returns "Trailing spaces "
Note: There are various white space characters not just the space. There are also special unicode characters the are visually 1 character (depending on device OS) yet take up 2 or more characters. eg "👨🚀".length === 5
is true
. All JavaScript strings are Unicode.
Rewrite
Using the same logic (build return string from array of split words) the following example attempts to correct the style and adherence to the requirements.
I prefer 4 space indentation (using spaces not tabs as tabs always seem to stuff up when copying between systems) however 2 spaces is acceptable (only by popularity)
I assume that the message
was converted from ASCII and spaces are the only white spaces of concern.
function crop(message, maxLength) { // use meaningful names
var result = message.trimEnd(); // Use var for function scoped variable
if (result.length > maxLength) // space between if ( > and )
const words = result.split(" "); // use const for variables that do not change
do
words.pop();
result = words.join(" ").trimEnd(); // ensure no trailing spaces
if (result.length <= maxLength) // not repeating same join operation
break;
while (words.length);
return result;
Note: Check runtime has String.trimEnd
or use a polyfill or transpiler.
Update $O(1)$ solution
I forgot to put in a better solution.
Rebuilding the string is slow, or passing the string through a regExp requires iteration over the whole string.
By looking at the character at the desired length you can workout if you need to move down to find the next space and then return the end trimmed sub string result, or just return the end Trimmed sub string.
The result has a complexity of $O(1)$ or in terms of $n = K$ (maxLength) $O(n)$
function crop(message, maxLength)
if (maxLength < 1) return ""
if (message.length <= maxLength) return message.trimEnd()
maxLength++;
while (--maxLength && message[maxLength] !== " ");
return message.substring(0, maxLength).trimEnd();
It is significantly faster than any other solutions in this question.
edited May 11 at 18:58
answered May 10 at 0:54
Blindman67Blindman67
11.2k1623
11.2k1623
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
add a comment |
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
regexp runtime is $O( K )$ too, just like your solution, and definitely does not iterate over the whole string. As to "significantly faster"—did your benchmark include the polyfill?
$endgroup$
– Oh My Goodness
May 11 at 12:58
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
May 11 at 19:35
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.
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%2f220011%2fcropping-a-message-using-array-splits%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