HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Filter functions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
functionsfilterstackoverflow

Problem

Here's a function I wrote - am I getting DRY about right? I could add another argument to paramFilter maybe, for even less reuse. Or have I gone too overboard as it is?

function goMageHack() {
    var ampFilter = function (str) {
        var amps = ["&", "amp%3B"];
        for (var j = 0; j < amps.length; j++) {
            str = fullReplace(amps[j],"amp;",str);
        }
        return str;
    }

    var paramFilter = function(element, param) {
        var $element = jQuery(element);
        for (var i = 0; i < param.length; i++) {
            if ($element.attr(param[i])) {
                $element.attr(param[i], ampFilter($element.attr(param[i])));
                $element.attr(param[i], fullReplace("amp;", "&", $element.attr(param[i])));
            }
        }
    }

    jQuery(".pages,.sort-by").find("a").each(function (_, element) {
        paramFilter(element, ["data-param", "href"]);
    });

    jQuery(".limiter").find("option").each(function(_, element){
        paramFilter(element, ["value"]);
    });
}


For completeness, since it is called from that function:

function fullReplace(needle, haystack, str) {
    str = String(str);
    var newStr;
    while ((newStr = str.replace(needle, haystack)) !== str) {
        str = newStr;
    }
    return newStr;
}


Addition

```
function handleChangingDropBox(attrib, that) {
var $this = jQuery(that);
var activeSet = Number($this.children(":selected").data("checkboxkey"));
var checkBoxes = checkBoxSets[activeSet];
var size;

if (currentlySelectedDropDownBox !== null) {
checkBoxSets[currentlySelectedDropDownBox].trigger("click");
}

currentlySelectedDropDownBox = activeSet;

if (size = checkBoxes.size()) {
var lastCheckBox = checkBoxes.last();
if (size !== 1) {
if ($this.val() === "choose") {
var dirtyParams = sp

Solution

I found something in your fullReplace function,

function fullReplace(needle, haystack, str) {
    str = String(str);
    var newStr;
    while ((newStr = str.replace(needle, haystack)) !== str) {
        str = newStr;
    }
    return newStr;
}


You can drop the while statement and write it like this

function fullReplace(needle, haystack, str) {
    str = String(str);
    str = str.replace(/needle/g, haystack)
    return str;
}


This will match every instance in the string because it is a global match. You can also do a case insensitive match if you need to.

Documentation for Replace

From what Flambino said I came up with this

var ampFilter = function (str) {
    return str.replace(/(&|amp%3B)/g, 'amp;');
}


This is where you originally called the fullReplace function. With this you shouldn't need to use the fullReplace function.

Code Snippets

function fullReplace(needle, haystack, str) {
    str = String(str);
    var newStr;
    while ((newStr = str.replace(needle, haystack)) !== str) {
        str = newStr;
    }
    return newStr;
}
function fullReplace(needle, haystack, str) {
    str = String(str);
    str = str.replace(/needle/g, haystack)
    return str;
}
var ampFilter = function (str) {
    return str.replace(/(&amp;|amp%3B)/g, 'amp;');
}

Context

StackExchange Code Review Q#51993, answer score: 2

Revisions (0)

No revisions yet.