4
\$\begingroup\$

I am looking for a better way to do a while cycle with the variable update.

The background is that I'm writing a StAX-line XML reader in JavaScript and want to implement a skipElement function. The function may only be called on the start tag and should skip everything until the closing end tag.

  • this.nextTag() skips to the next tag and returns Jsonix.XML.Input.START_ELEMENT or Jsonix.XML.Input.END_ELEMENT depending on whether it is a start or an end tag.
  • numberOfOpenTags holds the number of open tags, increased/decreased accordingly.
  • If we have a closing tag and numberOfOpenTags is 1, we're done.

skipElement : function() {
    if (this.eventType !== Jsonix.XML.Input.START_ELEMENT) {
        throw new Error("Parser must be on START_ELEMENT to skip element.");
    }
    // We have one open tag at the start
    var numberOfOpenTags = 1;
    // Skip to the next tag
    var et = this.nextTag();
    // If we have an END_ELEMENT and there is exactly one tag still open, we're done
    while (et !== Jsonix.XML.Input.END_ELEMENT || numberOfOpenTags !== 1)
    {
        if (et === Jsonix.XML.Input.START_ELEMENT)
        {
            numberOfOpenTags++;
        }
        else
        {
            numberOfOpenTags--;
        }
        et = this.nextTag();
    }
    return et;
}

I am looking for improvements. What I don't like:

  • this.nextTag() is called on two occasions.
  • Another check for et within the cycle: if (et === Jsonix.XML.Input.START_ELEMENT).
  • Too verbose if/else in the cycle.
\$\endgroup\$
4
\$\begingroup\$

How about don't go to the next element before the loop, just check for the counter to reach zero, get the next element first in the loop, and the use a conditional operator to determine how to change the counter:

skipElement : function() {
  if (this.eventType !== Jsonix.XML.Input.START_ELEMENT) {
    throw new Error("Parser must be on START_ELEMENT to skip element.");
  }
  var numberOfOpenTags = 1;
  var et;
  while (numberOfOpenTags > 0) {
    et = this.nextTag();
    numberOfOpenTags += et === Jsonix.XML.Input.START_ELEMENT ? 1 : -1;
  }
  return et;
}
\$\endgroup\$
  • \$\begingroup\$ Probably var et = this.eventType; is missing? \$\endgroup\$ – lexicore Oct 10 '14 at 23:43
  • \$\begingroup\$ @lexicore: Yes, you are right. However, I rewrote the code so that it returns the same element as the original code. \$\endgroup\$ – Guffa Oct 10 '14 at 23:44
  • \$\begingroup\$ Cool. I just added brackets around (et === Jsonix.XML.Input.START_ELEMENT). \$\endgroup\$ – lexicore Oct 10 '14 at 23:49
  • \$\begingroup\$ @lexicore: That's not needed as the comparison has the highest precedence here, but you might want it for readability. \$\endgroup\$ – Guffa Oct 10 '14 at 23:54
  • \$\begingroup\$ Correct, this was only for readability purposes. \$\endgroup\$ – lexicore Oct 10 '14 at 23:56

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.