4
\$\begingroup\$

I repeat here this answer on Stack Overflow.

I first posted an answer with not finalized code, as a simple description of the solution I could think, without any test. But later I remained interested, so I worked to make it (hopefully) perfectly functional.

To precisely define what it is meant to do, let me cite my previous answer:

This is a classic dilemma for any CMS or blog, where the teaser should present the begin of an article: often the solution is either stripping text from its tags and cut at a precise count OR keep tags but cut approximately because the tags are counted too...

So here the intent is to take an HTML element with any number of children, and any nesting level and return:

  • the "same" element (i.e. keeping its tag and attributes)
  • where resulting text content (i.e. visible as characters in the resulting page) is limited to a given count
  • where resulting text is built from successive text nodes in their natural order
  • where encountered tags are keeped intact and at their natural place

Here is my actual solution:

function cutKeepingTags(elem, reqCount) {
  var grabText = '',
      missCount = reqCount;
  $(elem).contents().each(function() {
    switch (this.nodeType) {
      case Node.TEXT_NODE:
        // Get node text, limited to missCount.
        grabText += this.data.substr(0,missCount);
        missCount -= Math.min(this.data.length, missCount);
        break;
      case Node.ELEMENT_NODE:
        // Explore current child:
        var childPart = cutKeepingTags(this, missCount);
        grabText += childPart.text;
        missCount -= childPart.count;
        break;
    }
    if (missCount == 0) {
      // We got text enough, stop looping.
      return false;
    }
  });
  return {
    text:
      // Wrap text using current elem tag.
      elem.outerHTML.match(/^<[^>]+>/m)[0]
      + grabText
      + '</' + elem.localName + '>',
    count: reqCount - missCount
  };
}

And here is a working example. (I kept the HTML example posted by the previous question OP)

\$\endgroup\$
6
\$\begingroup\$

First of all, I would just like to say that this is a really good and useful function. From a Code Review standpoint, there are almost no errors in it that I know of. Here are a few things I found from examining it:

Keep spacing uniform

Line 8 doesn't have a space between parameters, where your other function calls do. This is most likely due to quick typing and not any major issue other than a cleanliness nitpick.

An invalid input for if

Lines 19-21 where you have an if like so:

if (missCount == 0) {
  // We got text enough, stop looping.
  return false;
}

You should never use == over === due to it being possible that something like "0" would match the same as 0. This is because the double equals signs finds if it matches an exact value, where triple equals signs tests for exact value and type. So your final code should look like this for the if statement:

if (missCount === 0) {
  // We got text enough, stop looping.
  return false;
}

Optional - JSHint

If you use JSHint in JSFiddle, then you'll run into errors when trying to run this:

elem.outerHTML.match(/^<[^>]+>/m)[0]
  + grabText
  + '</' + elem.localName + '>',

If you're worried about that then you just have to write it all on one line. But for being short and concise, that might not be what you want.

\$\endgroup\$
  • \$\begingroup\$ Hey, this is unexpected: funnyly, from a stackOverflow question, I inadvertently put this answer in codeReview! I obviously agree with your first remark. But not for if (missCount == 0): I use to write a full test "===" only when really needed. Not the case here: missCount comes from reqCount then is only submitted to computations. So you made me realize that I rather should have controlled reqCount when entering function. Regarding JSHint diagnoses, I don't understand: I use to break expressions for readability, and don't see anything wrong here. Anyway, thanks for reviewing. \$\endgroup\$ – cFreed Oct 13 '15 at 20:34
  • \$\begingroup\$ With the if (missCount == 0) part, there is only a very rare case where you would want to use == over ===. Thus, it is best practice to use === all the time, and use the opposite if it's ever needed. It's a similar scenario to using single-quotes or double-quotes for strings. This lets you avoid a lot of coding errors when it may give you an unexpected result. Makes for much easier debugging. For the JSHint part, as I said, there is nothing wrong with your code. I was just saying that if you use JSHint available in JSFiddle, it'll give you an error unless you put it on a single line. \$\endgroup\$ – KingCodeFish Oct 13 '15 at 21:00
  • \$\begingroup\$ Also, if this answer helped you with cleaning up your code, please hit the check mark below the votes for this answer. That way people know it's resolved. \$\endgroup\$ – KingCodeFish Oct 13 '15 at 21:03
  • \$\begingroup\$ As explained above I'd not realized I was in codeReview, so I didn't even think to vote. But I do it now because yes, this answer helped me with cleaning up my code... in an unexpected way. I used JSHint to see the diagnoses you cited, then I tried it with my current app, just by curiosity. And it made me discover that, in a huge sequence of variables declarations after a unique var, I'd inadvertently closed a line by ";" instead of ",": so all following variables were declared in the global space. Thanks! \$\endgroup\$ – cFreed Oct 13 '15 at 21:48
  • \$\begingroup\$ Glad I was of service. :-) \$\endgroup\$ – KingCodeFish Oct 13 '15 at 21:51

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.