3
\$\begingroup\$

Given a text and the length limit I need to trim the text to the word which is nearest to the limit.

function trim(text, limit) {
  if (text.length < limit) return text;

  let end = -1;
  var i;
  for (i = 0; i < limit - 1; i++) {
    let current = text.charAt(i);
    let next = text.charAt(i + 1);
    if (current !== ' ' && next === ' ') {
      end = i;
    }
  }
  if (i === text.length - 1) {
    end = i;
  }

  return text.substring(0, end + 1);
}

[0, 2, 4, 6, 8, 10, 12, 50].forEach((limit) => {
  console.log(trim('Hello World', limit));
});

O/P:

//


Hello
Hello
Hello
Hello World
Hello World
\$\endgroup\$
4
\$\begingroup\$

The current code has a bug when the limit is exactly is exactly the length of a word. For example,

trim('Hello World', 5)

will return an empty string, when it should return "Hello", since it is of length 5, therefore it fits in the length limit.

The issue stems from the loop over i going from 0 to limit - 1 (excluded). The variable end represents the index of the last character to keep in the string, (because the method is returning text.substring(0, end + 1)). Therefore, it could be the case that end is equal to limit - 1, this happens when the limit falls right before a space. For example, when the limit is 5, and the string is 'Hello World', the last character to keep is 'o' at index 4. So the loop should go up to limit (excluded).

Then, there are slight adjustements to make:

  • The early-return testing text.length < limit should be text.length <= limit (because otherwise the next character in the loop over i won't exist for the last character of the string);
  • The added check if (i === text.length - 1) can safely be removed as a consequence: the only case where i would be equal to text.length - 1 is if limit == text.length (since i goes up to limit - 1 included). But that was already handled with the modified early-return.

There is a simpler and efficient way to tackle this problem, using the built-in lastIndexOf(searchValue, fromIndex). This method starts at the specified fromIndex, and searches backwards the first occurence of the given value. If we tell it to search the first occurence of ' ' searching backwards from limit, we'll exactly have the length of the substring to return.

The only corner case is if limit is greater than the length of the string, in which case the method should simply return the text unchanged (like the current code is doing). It needs to be tested separately; otherwise lastIndexOf would start searching from the end of the string, and the last word would always be cut. This is what it could look like:

function trim(text, limit) {
  if (limit >= text.length) {
    return text;
  }
  return text.substring(0, text.lastIndexOf(' ', limit));
}

[0, 2, 4, 5, 6, 8, 10, 11, 12, 50].forEach((limit) => {
  console.log(limit + ' -> ' + trim('Hello World', limit));
});

Note that, even if the lastIndexOf call returns -1, because there was no space character found before the limit length, it is still safe to call substring since:

If either argument is less than 0 [...], it is treated as if it were 0.

\$\endgroup\$
  • \$\begingroup\$ If you could help me with reducing off by one errors it would be good, do you have some method to avoid that? because I spend a considerable amount of time handling that. \$\endgroup\$ – CodeYogi Jan 7 '17 at 15:02
  • \$\begingroup\$ @CodeYogi Fair enough. I edited with the issues. \$\endgroup\$ – Tunaki Jan 7 '17 at 15:31
  • \$\begingroup\$ What if I have replicate the same logic in ruby since I find no such method in it? \$\endgroup\$ – CodeYogi Jan 8 '17 at 18:21
  • \$\begingroup\$ @CodeYogi I'm afraid I do not know Ruby at all... From what I searched, perhaps a combination of substring and rindex. \$\endgroup\$ – Tunaki Jan 8 '17 at 18:28

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.