On Jul 23, 6:42 am, sleepwalker <danielpaulr...@gmail.com> wrote:
> Hi,
> I have a old script that adds a class to a button/submit/reset based
> on the length of its value and I'd like to convert it to jquery. I'm
> just looking for any suggestions on the best way to rewrite it.

Instead of code, can you provide a concise statement of what the code
is expected to do? It may be different from what it actaully does.

> Thanks in advance for any help
>
> function setbuttonclass()
>         {
>                 if(document.getElementById && document.createTextNode)
>                         {
>                                 var x = 
> document.getElementsByTagName('input');

It doesn't seem sensible to infer the availability of a particular
method by testing a random selection of other methods.  To determine
if getElementsByTagName is supported, test it directly.


>                                 for (var i=0;i<x.length;i++)
>                                 {
>                                         var s = x[i].value;
>                                         var nolower = s.split(/[^A-Z\W]/);

If the intention is to split on lower case characters, why not:

  var nolower = s.split(/[a-z]/)

>                                         var word = (nolower.length/2 + 
> s.length);

Surely there is a simpler way to determine the class that is to be
applied to buttons?


>
>                                 if (((x[i].type == 
> ('button'||'reset'))||(x[i].type == 'reset')||(x

The expression - ('button'||'reset') - will always return the value
"button". When posting code, manually wrap it at about 70 characters
so that auto-wrapping doesn't introduce more errors.


> [i].type == 'submit')) && ((word) >= 12)){

The test to see if the input is of type button, submit or reset can be
done once, then the other logic applied. Getting x[i] every time in
every expression is very inefficient: store a reference and re-use it.

>                                                  x[i].className = 'mb';
>                                                                               
>                   }
>                                 else if (((x[i].type == 'button')||(x[i].type 
> == 'reset') ||(x
> [i].type == 'submit')) && ((word) <= 4) && (word != 0)){
>                                                   x[i].className = 'b';
>                                                                               
>                   }
>                                 else  {
>                                                 if (((x[i].type == 
> 'button')||(x[i].type == 'reset') ||(x
> [i].type == 'submit')) && (word != 0)){
>                                                          x[i].className = 
> 'sb';

Those tests seem out of sequence, and all the brackets make it very
hard to read. The code can be greatly simplified (see below).

>                                 }
>                         }
>                 }
>         }
> }

Consider:

function setButtonClass() {

  if (document && document.getElementsByTagName) {

    var inputs = document.getElementsByTagName('input');
    var reType = /button|reset|submit/;
    var reSplit = /[a-z]/;
    var i = inputs.length;
    var el, nolower, s, word;

    while(i--) {

      el = inputs[i];

      // Calculate word
      s = el.value;
      nolower = s.split(reSplit);
      word = (nolower.length/2 + s.length);

      // If input is of type button, reset or submit
      if (reType.test(el.type)) {

        // If word is 12 or greater, assign mb
        if (word > 11) {
          el.className = 'mb';

        // Else if word is less than 12 but greater than 4
        // assign sb
        } else if (word > 4 && word < 12) {
          el.className = 'sb';

        // Else if word is greater than zero (but less than 5)
        // assign b
        } else if (word > 0) { //
          el.className = 'b';
        }
        // If word less than 1, no className is assigned
      }
    }
  }
}


--
Rob

Reply via email to