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.

Answer 1

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.

Answer 2

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');
    },
}

Answer 3

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.

Answer 4

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?.