Adding and removing PHP parameters on click

Question

I'm trying to optimize this repetitive piece of code. I think I need to set subsection href as a variable and then figure out a way to merge the functions?

$(document).ready(function () {
    $('#alphabetical').click(function () {
        $('.subsection').attr('href', function () {
            return this.href + '&sort=alphabetical';
        });
    });
    $('#chronological').click(function () {
        $('.subsection').attr('href', function () {
            return this.href.replace('&sort=alphabetical', '');
        });
    });
    $('#builtfilter').click(function () {
        $('.subsection').attr('href', function () {
            return this.href + '&status=built';
        });
    });
    $('#allfilter').click(function () {
        $('.subsection').attr('href', function () {
            return this.href.replace('&status=built', '');
        });
    });
});

Answer 1

First off, you have a couple of bugs:

You can keep clicking the #alphabetical or #builtfilter buttons, and each time you do, it'll add the parameter. Even if it's already there. Yet the "reverse" buttons only replace/remove 1 instance. So if you have:

 /some/path?foo=bar

and click the alphabetical button again a couple of times, you get

 /some/path?foo=bar&sort=alphabetical&sort=alphabetical&sort=alphabetical

But you'll have to click #chronological an equal number of times, to "reset" everything, since only one &sort=alphabetical chunk is removed each time.

Secondly, I don't know the URL you're manipulating, but if there are no parameters to begin with, you end up with an invalid URL like:

/some/path&sort=alphabetical

Note that there's no ? to separate the query string from the path. Instead the & has ended up there - where it shouldn't be.

Also, if you URL has a fragment like #something, you'll be adding paramters after that, which won't work right. Like:

/some/path?foo=bar#fragment&sort=alphabetical

---

So first off, I'd recommend using jQuery's param method to create a well-formatted query string from an object:

var params = { sort: "", status: "built" }; // an example
var queryString = jQuery.param(params); //=> "sort=&status=built"

Secondly, you can use data-* attributes on your links to specify their action and classes to allow you to find all the links/buttons in one go (here's a related answer of mine).

For instance (simple example):

<button type="button" class="param" data-key="sort" data-value="alphabetical">Sort alphabetically</button>
<button type="button" class="param" data-key="sort" data-value="">Sort chronologically</button>

$(function () { // equivalent to $(document).ready(...)
  var params = { sort: "", status: "" }; // default params

  $(".param").on("click", function () {
    var $this = $(this),
        key = $this.data("param"),
        value = $this.data("value"),
        queryString;

    params[key] = value;
    queryString = jQuery.param(params);

    $(".subsection").attr("href", function () {
      // add or replace the query string as needed
      return this.href.replace(/(\?.*)?$/, "?" + queryString);
    });
  });
});

Note that this doesn't preserve fragments or any parameters that were already in the href, but not in the params object - but I'll leave that as an exercise.

Answer 2

Changes to your code:

• Enclosed it in an Immediately-Invoked Function Expression and "imported" the jQuery library.
• Added 'use strict' inside the IIFE.
• Used constants to denote magic values.
• Replaced $(document).ready() with jQuery's short hand version.
• Put everything inside one click event handler function.

The rest of it is pretty self-explanatory, I think. The code is a lot more verbose, but it makes everything a lot more flexible and DRY.

(function ($) {
    'use strict';

    var SUBSECTION_SLCTR    = '.subsection',
        ALPHABETICAL_SLCTR  = '#alphabetical',
        CHRONOLOGICAL_SLCTR = '#chronological',
        BUILTFILTER_SLCTR   = '#builtfilter',
        ALLFILTER_SLCTR     = '#allfilter';

    $(function () {
        var filterOptionSelectors = [
            ALPHABETICAL_SLCTR,
            CHRONOLOGICAL_SLCTR,
            BUILTFILTER_SLCTR,
            ALLFILTER_SLCTR
        ];

        $(filterOptionSelectors.join(', ')).click(function () {
            $(SUBSECTION_SLCTR).attr('href', function (i, value) {
                var $this = $(this);

                if ($this.is(ALPHABETICAL_SLCTR)) {
                    return value + '&sort=alphabetical';
                }

                if ($this.is(CHRONOLOGICAL_SLCTR)) {
                    return value.replace('&sort=alphabetical', '');
                }

                if ($this.is(BUILTFILTER_SLCTR)) {
                    return value + '&status=built';
                }

                // Remaining element is "all filter".
                return value.replace('&status=built', '');
            });
        });
    });
}(jQuery));

Note: Haven't had time to test this for errors.