Write function to loop over elements in jquery style library

Question

I am writing a very simple jquery imitation library, to allow for some simple DOM manipulation.

I am writing methods to allow me change color of text etc. When I wish to change a class element color I have to use a loop in each method. Ideally I would like to have a function that does that loop for me that I could then use in each method. Unfortunately, my attempt at this is not working.

Please see my code below:

function _(elem) {
  this.classOrId(elem);
}

_.prototype = {
  add: function(text) {
    if (this.e.length >= 1) {
      for (var i = 0; i < this.e.length; i++) {
        this.e[i].innerHTML = this.e[i].innerHTML + text;
      }
    } else {
      this.e.innerHTML = this.e.innerHTML + text;
    }
    return this;
  },
  replace: function(text) {
    if (this.e.length >= 1) {
      for (var i = 0; i < this.e.length; i++) {
        this.e[i].textContent = text;
      }
    } else {
      this.e.textContent = text;
      document.body.appendChild(elem);
    }
    return this;
  }
}

_.prototype.classOrId = function(elem) {
  var classOrId = elem.charAt(0);
  if (classOrId === "#") {
    elem = this.sliceElement(elem);
    this.e = document.getElementById(elem);
    return this;
  } else if (classOrId === ".") {
    elem = this.sliceElement(elem)
    this.e = document.getElementsByClassName(elem);
    return this;
  }
};

_.prototype.sliceElement = function(elem) {
  var elem = elem.slice(1);
  return elem;
};

As you can see there is an awful lot of repetition in this code. I tried writing the following to cut down on the repition but it didn't work. Any suggestions here would be greatly appreciated.

_.prototype.loopOverElements = function(effect) {
  for (var i = 0; i < this.e.length; i++) {
    return this.e[i][effect];
  }
}

With the code below it does not recognise the javascript DOM methods such as innerHTML, style when they are passed into the function.

With the code above if I pass the effect into the loopOverElements function it shows that console.log(this.e[i][effect]) is undefined when passed into the method.

what is loopOverElements supposed to do. Should it just loop through all the elements through the effect function? If so, I think this should just work - effect(this.e[i]); http://jsbin.com/buhezu/1/edit?html,js,console,output — blessanm86, Jan 09 '16 at 10:30
It is to be used to cut out the repition of me using a loop in each method of my code — Paul Fitzgerald, Jan 09 '16 at 10:33
no, unfortunately not :-( ... It must be possible, but maybe not.. what i wrote feels dirty, repeating that loop in each method... i dont like it — Paul Fitzgerald, Jan 09 '16 at 10:36
Can you explain a bit more. Like what is 'effect' parameter? Can you modify the demo I sent to use the 'loopOverElements' based on ur use case. After seeing how you want to use it will be more clear what needs to be done. — blessanm86, Jan 09 '16 at 10:52
parameter should be an effect such as `style.fontSize` or `style.backgroundColor`. I have updated the jsfiddle with the complete code - http://jsbin.com/lahikotojo/edit?html,js,console,output — Paul Fitzgerald, Jan 09 '16 at 10:53
Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/100224/discussion-between-paul-fitzgerald-and-blessenm). — Paul Fitzgerald, Jan 09 '16 at 10:54

Shanoor · Accepted Answer · 2016-01-09T12:02:15.403

You should probably post this on codereview but I'll give it a shot. The first thing bothering me is all the if (this.e.length >= 1) { } else { }. Make e an array even when there is only one element, only a variable's value should "variate" not its type.

About classOrId(), it can be reduced to one line thanks to document.querySelectorAll (returning the needed array). So the code becomes:

function _(elem) {
    var about = {
        Name: "pQuery",
        Version: 0.1,
        Author: "Paul Fitzgerald"
    }

    if (elem) {
        if (window === this) {
            return new _(elem);
        }
        this.e = document.querySelectorAll(elem); // no need for classOrId() anymore
    } else {
        return about;
    }
}

_.prototype = {
    add: function (text) {
        // no if else anymore
        for (var i = 0; i < this.e.length; i++) {
            this.e[i].innerHTML = this.e[i].innerHTML + text;
        }
        return this;
    }
}

About the repetition, separate the functions into similar usages, for example you could write a function for all the operations related to style:

_.prototype = {
    _eachStyle: function (prop, value) {
        for (var i = 0; i < this.e.length; i++) {
          this.e[i].style[prop] = value;
        }

        return this;
    },
    hide: function () {
        return _eachStyle('display', 'none');
    },
    color: function (color) {
        return _eachStyle('color', color);
    }
}

For the base properties:

_.prototype = {
    _each: function (prop, value, append) {
        append = append || false; // by default, replace the value
        for (var i = 0; i < this.e.length; i++) {
          this.e[i][prop] = append ? element[prop] + value : value;
        }
        return this;
    },
    add: function (text) {
        return _each('innerHTML', text, true);
    },
    replace: function (text) {
        return _each('textContent', text);
    }
}

Similarely, for a function to be called on all elements:

_.prototype = {
    _eachFn: function (fn, args) {
        for (var i = 0; i < this.e.length; i++) {
          this.e[i][fn](args);
        }
        return this;
    },
    remove: function () {
        return _eachFn('remove');
    },
}

@PaulFitzgerald Weird, `e` is an array (empty if no element found) so `forEach()` is always there unless you're using a very old browser. — Shanoor, Jan 09 '16 at 11:30
@PaulFitzgerald Yes if I set `this.e` to a static array. I found the issue, querySelectorAll returns a NodeList object not an array. I think a simple for should work, I'm doing some tests. — Shanoor, Jan 09 '16 at 11:47
@PaulFitzgerald, so it works with a simple for loop. I updated my answer. There is still room for improvement (_each and _eachStyle are close) but it's a good start. — Shanoor, Jan 09 '16 at 12:03

score 1 · Answer 2 · answered Jan 09 '16 at 12:04

As I mentioned earlier in the chat session, you basically want to apply transformations on each DOM element in a array. Using a function that defines what needs to be done and mapping each element through this function is the ideal approach.

_.prototype.loopOverElements = function(effect) {
  for (var i = 0; i < this.e.length; i++) {
    effect(this.e[i]);
  }
};

And I can use it as

var test = new _('.test');
test.loopOverElements(function(elem) {
  elem.innerHTML += " Modified";
});

Here is a working demo for this implementation.

But your requirement is that you want a loop method that has 2 parameters -propertyName and value. There are many problems associated with this like suppose the property is style.color. You cannot access that property using this.e[i]['style.color']. Its not valid js. It should be this.e[i]['style']['color']. So basically this.e[i][effect] will not work in this case. Library's like lodash have a _.set method where you can specify a property path and the value will be correctly set. You can see the implementation here.

I've created a naive implementation of the set method.

_.prototype.set = function set(obj, path, value) {
  var codeString = 'obj';
  codeString += path.split('.').reduce(function(prev, curr) {
    return prev += '.' + curr;
  }, '');

  codeString += ' = value';
  eval(codeString);
};

Now your loop code looks like

_.prototype.loopOverElements = function(property, value){
  for(var i = 0; i < this.e.length; i++){
    this.set(this.e[i], property, value);
  }
};

And it can be used like

document.addEventListener("DOMContentLoaded", function(event) {
  var test = new _('.testClass');
  test.loopOverElements('style.color', 'red');
});

Here is a working demo for this implementation.

score 0 · Answer 3 · edited May 23 '17 at 10:28

0

Use brackets notation, i.e. [effect] instead of .effect:

_.prototype.loopOverElements = function(effect){
  for(var i = 0; i < this.e.length; i++){
    return this.e[i][effect];
  }
}

See also JavaScript property access: dot notation vs. brackets?.

edited May 23 '17 at 10:28

Community

1
1

answered Jan 09 '16 at 10:15

Michał Perłakowski

88,409
26
156
177

for this when I pass in an 'effect' it says `innerHTML` is not defined – Paul Fitzgerald Jan 09 '16 at 10:19
@PaulFitzgerald What do you get when you `console.log(this.e[i])`? Also, are you passing `innerHTML` as parameter, or string `"innerHTML"`? – Michał Perłakowski Jan 09 '16 at 10:20
if i pass it in as a string it returns the html array of classes, but doesn't perform th update to the elements styling – Paul Fitzgerald Jan 09 '16 at 10:27
if you would like to play around with the whole code in your text editor you can find it here : `https://github.com/Pau1fitz/pQuery/blob/master/index.html` I always find it easier to answer questions on this by doing this and looking at the code in my own text editor – Paul Fitzgerald Jan 09 '16 at 10:29

score 0 · Answer 4 · answered Jan 09 '16 at 11:24

Another approach to solve your problem is using a callback function:

_.prototype.applyOnElements = function(callback) {
   for(var i = 0; i < this.e.length; i++){
     callback(this.e[i]);
   }
}

And you use it this way:

this.applyOnElements(function(element) {
    element.style.color = myColor;
});

Then there is place to improvement by replacing the loop by Array.forEach and including your if (this.e is array) then ... else ... endif inside this function to avoid more code duplication.

Write function to loop over elements in jquery style library

4 Answers4