5
\$\begingroup\$

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;
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Is this a function? \$\endgroup\$ – 200_success Jun 14 '17 at 15:03
  • \$\begingroup\$ Instead of (p > 0 && (str[p] !== " " && str[p] !== "-")), you can try p=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 capped p to 16? \$\endgroup\$ – Rajesh Jun 15 '17 at 7:52
  • 1
    \$\begingroup\$ Unfortunately updating your post with new, updated, code is against site rules. For more information see what you may and may not do after receiving answers. \$\endgroup\$ – Peilonrayz Jun 15 '17 at 14:41
2
\$\begingroup\$

Some minor changes to improve readability:

  • It's generally considered better practice to use str.charAt(0) instead of str[0] (even though str[0] often outperforms .charAt()).
  • Simplify the while condition and have it only check if p > 0; the character checks can be done inside the loop. This also lets you eliminate the if (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.

\$\endgroup\$
  • 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 Jun 15 '17 at 14:25
0
\$\begingroup\$

@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);
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.