2
\$\begingroup\$

I am writing a (multi) filter that is created when the user enters in the data through wordpress. There can be anywhere between 2 - 10 filters so I need this to be as dynamic as possible. My first approach was to write out a boolean for each possible filter being created and check if it is there or not. Inside the boolean is a click function that checks if the data === "yes" and then add a active class to the item. I rewrote it in a do while, but am not sure if this is the best approach or if they are both bad.

I am not looking for a complete answer or working example, but rather if my way makes sense. If so, why? If not, why not? And an explanation of how I could possible improve it or think of it in a different way.

Manually check

var mem = $(".member");

var filter1 = $(".filter-1");
var filter2 = $(".filter-2");
var filter3 = $(".filter-3");

var memCom1 = $(".member").data("com-1");
var memCom2 = $(".member").data("com-2");
var memCom3 = $(".member").data("com-3");

//  DO THIS FOR ALL POSSIBLE

if (memCom1.length > 0 || filter1.length > 0) {
    filter1.click(function() {
        var com1 = $(this).data("com-1");
        if(com1 === "yes") {
            $(this).addClass(active);
        }
    });
}

if (memCom2.length > 0 || filter2.length > 0) {
    filter2.click(function() {
        var com2 = $(this).data("com-2");
        if(com2 === "yes") {
            $(this).addClass(active);
        }
    });
}

if (memCom3.length > 0 || filter3.length > 0) {
    filter3.click(function() {
        var com3 = $(this).data("com-3");
        if(com3 === "yes") {
            $(this).addClass(active);
        }
    });
}

// would go up to 10

Do while

var mem = $(".member");
var active = "is-active"

do {
    var i = 1;
    if ( $(".member").data("com-"+i).length > 0 || $(".filter"+i).length > 0 ) {
        $(".filter-"+i).click(function() {
            if( $(this).data("com-"+i) === "yes" ) {
                $(this).addClass(active);
            }
        });
    }
  i++;
}

while (i <= mem.length);
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

The first set of code, while it's straightforward, isn't "ideal" in a sense that you are repeating code. You violate the DRY principle, so let's scratch that for your second code instead.

Now I'm not sure how data is set to the data property. I assume your filters are some kind of custom checkbox which sets com-* data attributes (since your checks are a "yes or not yes").

if (memCom3.length > 0 || filter3.length > 0)

Now as far as I understand, this condition is used only to conditionally attach the click handler. From what I see, the second condition is not required. jQuery objects are collections and jQuery methods iterate through and operate through each. If the collection is empty, the operation does nothing since it iterates through nothing. So your condition can be simplified to

$('.filter').on('click', function(){
  // What happens to each filter
});

One more touch. Your click depends on a certain number. Now if you could just extract that number by itself, possibly using a data-* attribute on the filter.

$(".filter").on('click', function(){
  var element = $(this);
  var number = element.data('number');
  var com = element.data('com-' + number);
  var memberData = $(".member").data('com-' + number).length;
  if(!(com === 'yes' && memberData)) return;
  $(this).addClass(active);
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.