CODEWITHSUNDEEP
javascriptjqueryrefactoring
John Smith
John Smith

Reputation: 8811

How to refactor these three functions in to one?

There is only a one line difference and was wondering if it was at all possible!

function sortHot() {
    var order = [];

    $("#container").children(".submission-preview").each(
        function() {
            var key = $(this).attr('id');

            var value = $("div#" + key.replace("sub", "votes")).html() * key.replace("sub", "");

            order.push([key, value]);
        }
    );

    return order;
}

function sortNew() {
    var order = [];

    $("#container").children(".submission-preview").each(
        function() {
            var key = $(this).attr('id');

            var value = key.replace("sub", "");

            order.push([key, value]);
        }
    );

    return order;
}

function sortTop() {
    var order = [];

    $("#container").children(".submission-preview").each(
        function() {
            var key = $(this).attr('id');

            var value = $("div#" + key.replace("sub", "votes")).html();

            order.push([key, value]);
        }
    );

    return order;
}

Upvotes: 2

Views: 131

Answers (4)

Guffa
Guffa

Reputation: 700252

If you want to keep the function names, you can move everything but the differing line into a function, and call it with a function that returns the value.

You can use the map method instead of each, as it's intended for creating an array from a jQuery result:

function sort(f) {
  return $("#container").children(".submission-preview").map(
    function() {
      var key = $(this).attr('id');
      return [key, f(key)];
    }
  ).get();
}

function sortHot() {
  return sort(function(key){
    return $("div#" + key.replace("sub", "votes")).html() * key.replace("sub", "");
  });
}

function sortNew() {
  return sort(function(key){
    return key.replace("sub", "");
  });
}

function sortTop() {
  return sort(function(key){
    return $("div#" + key.replace("sub", "votes")).html();
  });
}

If you want to have a single function, then you can reuse the parts of the differing lines that use the same values:

function sort(mode) {
  return $("#container").children(".submission-preview").map(
    function() {
      var key = $(this).attr('id');
      var value = key.replace("sub", "");
      if (mode != 'New') {
        var html = $("div#" + key.replace("sub", "votes")).html();
        value = mode == 'Hot' ? html * value : html;
      }
      return [key, value];
    }
  ).get();
}

Upvotes: 1

scoffey
scoffey

Reputation: 4688

A more "functional" way of generalizing the key-to-value conversion is to wrap a generic sort function that accepts a callback argument for applying that transformation on the key. For example:

function sortGeneric(callback) {
    var order = [];
    $("#container").children(".submission-preview").each(function() {
        var key = $(this).attr('id');
        var value = callback(key);
        order.push([key, value]);
    });
    return order;
}

function sortHot() {
    return sortGeneric(function (key) {
        return $("div#" + key.replace("sub", "votes")).html() * key.replace("sub", "");
    });
}

function sortNew() {
    return sortGeneric(function (key) {
        return key.replace("sub", "");
    });
}

function sortTop() {
    return sortGeneric(function (key) {
        return $("div#" + key.replace("sub", "votes")).html();
    });
}

Upvotes: 4

Rafal
Rafal

Reputation: 2576

You can add a parameter and do an if statement

function sortTop(type) {
    var order = [];

    $("#container").children(".submission-preview").each(
        function() {
            var key = $(this).attr('id');

            if(type == 'top'){
               var value = $("div#" + key.replace("sub", "votes")).html();
            }else  if(type == 'hot'){
               var value = $("div#" + key.replace("sub", "votes")).html() * key.replace("sub", "");
            }else{
               var value = key.replace("sub", "");

            }

            order.push([key, value]);
        }
    );

    return order;
}

Upvotes: 0

BoltClock
BoltClock

Reputation: 723528

You could use a switch with an argument passed to the function:

function sortBy(sortOption) {
    var order = [];

    $("#container").children(".submission-preview").each(
        function() {
            var key = $(this).attr('id');
            var value;

            switch (sortOption) {
                case "hot":
                    value = $("div#" + key.replace("sub", "votes")).html() * key.replace("sub", "");
                    break;

                case "new":
                    value = key.replace("sub", "");
                    break;

                case "top":
                    value = $("div#" + key.replace("sub", "votes")).html();
                    break;

                // You may want to provide a default case here, or
                // have one of the above options be the default
            }

            order.push([key, value]);
        }
    );

    return order;
}

Then call it like this, for the three respective options:

sortBy("hot");
sortBy("new");
sortBy("top");

Upvotes: 3

Related Questions