cropping a message using array splitsSimplify code using arrayMilliseconds to Time string & Time string to MillisecondsSlideshow by array using slice methodDefinitional Returns. Solved. MostlyA String.prototype.diff() implementation (text diff)Using promise all with push arraySmall JavaScript library for ECMAScript version detectionThis macro will convert you… or notReturns Google Sheet cells from a named rangeImporting text into PANDAS and counting certain words
What does this quote in Small Gods refer to?
What was the notion of limit that Newton used?
Is there an application which does HTTP PUT?
Exception propagation: When to catch exceptions?
Is there any evidence to support the claim that the United States was "suckered into WW1" by Zionists, made by Benjamin Freedman in his 1961 speech
How to slow yourself down (for playing nice with others)
Why are low spin tetrahedral complexes so rare?
How to find the tex encoding of specific fonts?
How to evaluate sum with one million summands?
Was there a contingency plan in place if Little Boy failed to detonate?
Can 'sudo apt-get remove [write]' destroy my Ubuntu?
Is a vertical stabiliser needed for straight line flight in a glider?
Is it a Munchausen Number?
Is every story set in the future "science fiction"?
Remove color cast in darktable?
Is this state of Earth possible, after humans left for a million years?
Company threw a surprise party for the CEO, 3 weeks later management says we have to pay for it, do I have to?
What does formal training in a field mean?
What was the plan for an abort of the Enola Gay's mission to drop the atomic bomb?
Does the 500 feet falling cap apply per fall, or per turn?
Why is the Sun made of light elements only?
Would encrypting a database protect against a compromised admin account?
Why did Captain America age?
Why is PerfectForwardSecrecy considered OK, when it has same defects as salt-less password hashing?
cropping a message using array splits
Simplify code using arrayMilliseconds to Time string & Time string to MillisecondsSlideshow by array using slice methodDefinitional Returns. Solved. MostlyA String.prototype.diff() implementation (text diff)Using promise all with push arraySmall JavaScript library for ECMAScript version detectionThis macro will convert you… or notReturns Google Sheet cells from a named rangeImporting 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 array
$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 array
$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 array
$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 array
javascript performance array
asked 4 hours ago
beNerdbeNerd
1273
1273
add a comment |
add a comment |
3 Answers
3
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$
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$
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.
$endgroup$
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
3 Answers
3
active
oldest
votes
3 Answers
3
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$
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$
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 3 hours ago
Oh My GoodnessOh My Goodness
2,607315
2,607315
add a comment |
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$
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$
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 2 hours ago
answered 2 hours ago
EmmaEmma
2351215
2351215
add a comment |
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.
$endgroup$
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.
$endgroup$
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.
$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.
answered 19 mins ago
Blindman67Blindman67
10.6k1623
10.6k1623
add a comment |
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