I have to wrap my text, when it's longer then 16 characters and when there is space " " or a dash "-". Can I write it with regex or a function that is easier to read than mine below?
wrapType(str) {
if (str.length > 16) {
var p = 16;
while (p > 0 && (str[p] !== " " && str[p] !== "-")) {
p--;
}
if (p > 0) {
var left = (str.substring(p, p + 1) === "-") ? str.substring(0, p + 1) : str.substring(0, p);
var right = str.substring(p + 1);
return left + "\n" + this.wrapType(right);
}
}
return str;
}
2 Answers 2
Some minor changes to improve readability:
- It's generally considered better practice to use
str.charAt(0)
instead ofstr[0]
(even thoughstr[0]
often outperforms.charAt()
). - Simplify the
while
condition and have it only check ifp > 0
; the character checks can be done inside the loop. This also lets you eliminate theif (p > 0)
check. - Add some comments.
You could also incorporate p--
into your while
loop (so it would read while (p-- > 0) {
but that wouldn't improve readability.
wrapType(str) {
if (str.length > 16) {
// if string is longer than 16 characters
var p = 16;
while (p > 0) {
// loop through the string and check each char
if ((str.charAt(p) === " " || str.charAt(p) === "-") {
// if a dash or space is found, wrap the string
var left = (str.charAt(p) === "-") ? str.substring(0, p + 1) : str.substring(0, p);
var right = str.substring(p + 1);
return left + "\n" + this.wrapType(right); // return the wrapped string
}
p--;
}
}
return str; // return the original string
}
If you wanted to take it further, you could create a small function to do the character-checking called breakHere
or something similar that would return true
if it found a dash or space, and false
otherwise. Then for your if
checks, instead of checking for both the dash and space, you could just do something like if (breakHere()) { ... }
which would make for slightly longer code but might make it easier for someone new to follow what's going on. You could even have that function return the value itself, so you could call newStr = breakHere();
and newStr
would either be assigned the wrapped string (if a dash or space was found) or the original string (str
). That would allow you to combine your two separate return
statements into one -- return newStr
.
-
1\$\begingroup\$ Thank you. I would like to use it, but had to add another function, to look for space or dash after the 16 (my maxLength) - so it wouldn't work with the if in the while. I split it in 5 function, because of the max complexity. \$\endgroup\$Meyra– Meyra2017年06月15日 14:25:59 +00:00Commented Jun 15, 2017 at 14:25
@freginold thank you for your review but I couldn't use this, because I had to add a function which checks if the string has space or dash after the 16th char. This was not included in my question.
I split this whole thing in 5 functions because of the maxComplexity. Now i made a new class for it and it looks like this:
class TextWrapper {
wrapType(str, maxL) {
if (str.length > maxL) {
return this.wrapBeforeMaxLength(str, maxL);
}
return str;
}
wrapBeforeMaxLength(str, maxL) {
var p = maxL;
while (p > 0 && !this.containsSeparator(str.charAt(p))) {
p--;
}
return p > 0 ? this.wrapTheText(str, p, maxL) : this.wrapAfterMaxLength(str, maxL);
}
wrapAfterMaxLength(str, maxL) {
if (!this.containsSeparator(str)) {
return str;
}
var p = maxL + 1;
while (p < str.length && !this.containsSeparator(str.charAt(p))) {
p++;
}
return this.wrapTheText(str, p, maxL);
}
containsSeparator(str) {
return _.includes(str, " ") || _.includes(str, "-");
}
wrapTheText(str, p, maxL) {
if (0 < p && p < str.length) {
var left = (str.substring(p, p + 1) === "-") ? str.substring(0, p + 1) : str.substring(0, p);
var right = str.substring(p + 1);
return left + "\n" + this.wrapType(right, maxL);
}
}
}
function
? \$\endgroup\$(p > 0 && (str[p] !== " " && str[p] !== "-"))
, you can tryp=16 - str.match(/[^- ]/g).length
. But what if the string is of length 100 and there are total 40 spaces or dashes? And why have you cappedp
to 16? \$\endgroup\$