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
Why do the lights go out when someone enters the dining room on this ship?
Will the volt, ampere, ohm or other electrical units change on May 20th, 2019?
Holding rent money for my friend which amounts to over $10k?
Is there an academic word that means "to split hairs over"?
Is it wrong to omit object pronouns in these sentences?
Do not cross the line!
Why did the soldiers of the North disobey Jon?
Acronyms in HDD specification
How can I add a .pem private key fingerprint entry to known_hosts before connecting with ssh?
A case where Bishop for knight isn't a good trade
Is it safe to use two single-pole breakers for a 240 V circuit?
Why are BJTs common in output stages of power amplifiers?
How to make a not so good looking person more appealing?
Developers demotivated due to working on same project for more than 2 years
What information exactly does an instruction cache store?
How about space ziplines
Should generated documentation be stored in a Git repository?
What is the correct pentalobe screwdriver size for a Macbook Air Model A1370?
Biology of a Firestarter
Is this possible when it comes to the relations of P, NP, NP-Hard and NP-Complete?
Use of さ as a filler
Can only the master initiate communication in SPI whereas in I2C the slave can also initiate the communication?
Single word that parallels "Recent" when discussing the near future
What is this old US Air Force plane?
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
1624
1624
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) [ "" ] )[0];
.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
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
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) [ "" ] )[0];
.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) [ "" ] )[0];
.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) [ "" ] )[0];
.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) [ "" ] )[0];
.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,791316
2,791316
$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
2651215
2651215
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
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
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
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
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 2 days ago
answered May 10 at 0:54
Blindman67Blindman67
10.8k1623
10.8k1623
$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
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
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
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
$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
2 days ago
$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
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
$begingroup$
@AuxTaco Oh dear, my bad I managed to not submit a previous edit, fixed it
$endgroup$
– Blindman67
2 days ago
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