I am currently using the code below to truncate text:

 function truncate($elem, wordNum){

var words = $elem.find('p').text().trim().split(/\s+/).slice(0,wordNum);
var replacement = $("<p/>").text(words.join(" ") + "...");
   if($(this).text() == '
   } else if($(this).text() == 'Read Less'){
    return old=='Read More' ?  'Read Less' : 'Read More';
return false


truncate($("p"), 5);

Works, but it's not the most efficient solution. Is there a better way ?

  • \$\begingroup\$ What's not efficient? BTW you might want to use \s+ to match multiple spaces. \$\endgroup\$
    – woxxom
    Commented Jan 26, 2017 at 12:40
  • \$\begingroup\$ Still improving it, but so far for each element I am using multiple truncate function calls. Ah yes good shout. \$\endgroup\$ Commented Jan 26, 2017 at 12:59
  • \$\begingroup\$ Which of these code snippets are we supposed to be reviewing? If the second one is your current code and an improvement on the first code, I think you should just remove the first code altogether. \$\endgroup\$
    – Mike Brant
    Commented Jan 26, 2017 at 15:39
  • \$\begingroup\$ mike will do now sorry, new to this. \$\endgroup\$ Commented Jan 26, 2017 at 15:55

1 Answer 1


A few thoughts:

  • /\s/ is probably not the best thing to be searching for. I would consider using /\b/ to look for all word boundaries, not just spaces.
  • It seems the $elem here would be a jQuery collection, however you are treating it is a single element without using things such a $.each() to be able to apply to multiple elements. Your function fails if more than one element is passed in $elem. This is a bad way to think about writing code against jQuery. You should be thinking about working against collections, not single elements.
  • Since you are using jQuery, I would strongly suggest writing this as a jQuery plug-in such that you can better attach the behavior to a collection of elements (i.e. $('.some_class').truncatable({count: ...});.
  • Is word count really meaningful as a way to determine how you are truncating? I would think better logic would be to look for first word boundary at a location less than X number of characters in string as your truncation point. That way you get more consistent size of truncated text, not subject to vary greatly based on overly long or overly short words in the content.
  • In the second example, it seems odd to hard-code nested <p>, .read_more, etc. into the function. Why would $elem not already be whatever element you want to work with (the <p> in this case)? What if in the future you wanted to apply this to a <div> that does not have a <p> nested inside of it? Your code will break. You are limiting the usefulness of your code in doing this.
  • You should get in the habit of using strict comparisons (===, !==) by default instead of loose comparisons (==, !=). This will make your code less fragile to unexpected truthy/falsey behavior, and this easier to maintain/debug. I would say the loose comparisons should only be used when you have a very specific use case for doing so and would ideally include a comment in such cases explaining why such loose comparison is appropriate for that case.
  • I question the approach of storing the full/truncated texts in an alternate DOM elements. I would think that perhaps when this truncation behavior is applied to an element, that you might want to store the full text (and perhaps the truncated text as well, if you want to avoid recalculation) in a data attribute. If you take this approach, I would suggest using $.text() or native javascript textContent as means to make this change on click vs. $.html() or innerHTML for performance reasons. You definitely don't want to be inserting nodes in DOM using $.before() here.
  • I know this is code review and we are reviewers should focus on the code presented, but I would suggest that you think about whether you are re-inventing the wheel here. There are several plug-ins for jQuery that do this already - https://plugins.jquery.com/tag/truncate/
  • \$\begingroup\$ Interesting points. Could you point me in the right direction to append your recommendations to my code ? \$\endgroup\$ Commented Jan 26, 2017 at 16:22
  • 1
    \$\begingroup\$ In JS \b works only on ASCII and fails on unicode so it's better to use an explicit punctuation character enumeration, I guess. \$\endgroup\$
    – woxxom
    Commented Jan 26, 2017 at 17:11
  • \$\begingroup\$ @wOxxOm Very good point. Google search on this issue would present a number of approaches to addressing this problem if you are concerned with unicode compatibility - google.com/… \$\endgroup\$
    – Mike Brant
    Commented Jan 26, 2017 at 18:23
  • \$\begingroup\$ @VaishalPatel I would consider starting from scratch or looking at using an existing plug-in, as if you follow my recommendations, what you end up with could look close to 100% different than what you have. If you are going to rewrite, post an iterative code review here as new question (and perhaps link to this). \$\endgroup\$
    – Mike Brant
    Commented Jan 26, 2017 at 18:26
  • \$\begingroup\$ Mike Brant shall do \$\endgroup\$ Commented Jan 27, 2017 at 9:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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