Loading ...
Sorry, an error occurred while loading the content.

1617Re: Filtered for...in

Expand Messages
  • Carlos Vadillo
    Nov 18, 2010
    • 0 Attachment
      --- In jslint_com@yahoogroups.com, Marcel Duran <contact@...> wrote:
      >
      > Here's an old function from a greasemonkey script I wrote a few years ago
      > that uses label and break instead of a bunch of control variables to
      > properly break the outer loop when searching for selectors in stylesheets:
      >
      > var changeCSS = function (selector, property, value) {
      > var i, j, lenR, rules, found,
      > doc = document,
      > styleSheets = doc.styleSheets,
      > lenS = styleSheets.length,
      > jsProperty = property.replace(/-([\s\S])/g, function () {
      > return arguments[1].toUpperCase();
      > });
      >
      > // get selector and add/change style
      > css:
      > for (i = 0; i < lenS; i += 1) {
      > rules = styleSheets[i].cssRules;
      > for (j = 0, lenR = rules.length; j < lenR; j += 1) {
      > if (rules[j].selectorText === selector) {
      > found = true;
      > rules[j].style[jsProperty] = value;
      > break css;
      > }
      > }
      > }
      >
      > // add selector when not found
      > if (!found) {
      > styleSheets[i - 1].insertRule(selector + '{' + property + ':' +
      > value + ';}', j - 1);
      > }
      > };
      >
      > And it's JSLint approved.
      >
      >
      > Marcel
      >
      > On Wed, Nov 17, 2010 at 6:35 PM, Michael Mikowski <z_mikowski@...>wrote:
      >
      > >
      > >
      > > Actually, Douglas,
      > >
      > > My standards do follow your recommendations about filtering; unfortunately,
      > > the
      > > example I contrived does not.
      > > The intent there was to show the use of a label for continue and break. It
      > > was
      > > another poster who wanted to use continue as a filter method.
      > >
      > > Someone else mentioned that "goto is evil". Yes, but the labels are not
      > > intended for use with goto. They are intended to mark a loop so that if one
      > >
      > > uses continue or break, its obvious what control structure is affected.
      > >
      > > Cheers, Mike
      > >
      > >
      > > ________________________________
      > > From: Douglas Crockford <douglas@... <douglas%40crockford.com>>
      > > To: jslint_com@yahoogroups.com <jslint_com%40yahoogroups.com>
      > > Sent: Wed, November 17, 2010 10:35:50 AM
      > >
      > > Subject: [jslint] Re: Filtered for...in
      > >
      > > --- In jslint_com@yahoogroups.com <jslint_com%40yahoogroups.com>, Michael
      > > Mikowski <z_mikowski@> wrote:
      > > >
      > > > In my team code standards, continue and break are only to be used with a
      > > label.
      > > >
      > > > This is similar to other Best Practice conventions in other languages
      > > because
      > >
      > > > it avoids the ambiguity of the continue or break statement.
      > >
      > > The purpose of JSLint is not to make you feel good about inadequate coding
      > > standards. Since you are refusing to consider my advice to avoid continue,
      > > then
      > > I recommend that you turn on the "Tolerate unfiltered for in" option.
      > >
      > > [Non-text portions of this message have been removed]
      > >
      > >
      > >
      >
      >
      >
      > --
      > Marcel Duran
      >
      >
      > [Non-text portions of this message have been removed]
      >

      I think you can always write an alternative code that conveys better your intentions:


      var changeCSS = function (selector, property, value) {
      var i,
      rules,
      found,
      doc = document,
      styleSheets = doc.styleSheets,
      lenS = styleSheets.length,
      jsProperty = property.replace(/-([\s\S])/g, function () {
      return arguments[1].toUpperCase();
      }),
      findSelectorInRules = function (rules) {
      var j,
      lenR;

      for (j = 0, lenR = rules.length; j < lenR; j += 1) {
      if (rules[j].selectorText === selector) {
      rules[j].style[jsProperty] = value;
      return true;
      }
      }
      return false;
      },
      findSelectorInStyleSheets = function (styleSheets) {
      var i,
      rules,
      lenS = styleSheets.length;

      for (i = 0; i < lenS; i += 1) {
      rules = styleSheets[i].cssRules;
      if (findSelectorInRules(rules)) {
      return true;
      }
      }
      return false;
      },
      insertRuleAtEnd = function (styleSheets) {
      var lenS = styleSheets.length - 1,
      lenR = styleSheets[lenS].cssRules.length - 1;

      styleSheets[lenS].insertRule(selector + '{' + property + ':' + value + ';}', lenR);
      };

      // get selector and add/change style
      if (!findSelectorInStyleSheets(styleSheets)) {
      insertRuleAtEnd(styleSheets);
      }
      };

      As I understood the code it only has one function, to insert a selector that does not exist. Then the function only have two statements: The first to determine if the selector is present in the styleSheets and the second to insert the selector at the end of the arrays.

      By breaking your code into nested functions what you gain is a piece of code that can be read by a human: If I cannot find the selector in the current style sheets then I will insert it at the end. How you find the selector is not important at the narrative of the function. Then you go to next level of functions, and have a narrative also: review all the styleSheets and find the selector in their rules. So on and so forth.

      No need for continue, no need for break, no need for labels and a code that is easy to read
    • Show all 19 messages in this topic