1
\$\begingroup\$

I have a button group that looks like this:

Button group

The user selects one of the options and they can search for a person based on that criteria.

I wrote a switch statement that populates the URL to make the ajax call to get the data based on the option selected.

However, the down side of this is that every time an option is added or removed, I have to modify the corresponding JavaScript.

Should I re-factor the code to use data- attributes on the a tags, that contain the URL to use?

        <li><a href="#" id="btUsername" data-url="SearchByUsername">Username</a></li>
        <li><a href="#" id="btLastName" data-url="SearchByLastName">Last Name</a></li>
        <li><a href="#" id="btStudentID" data-url="SearchByStudentID">Student ID</a></li>

Or is that considered bad mojo?


What I have working:

HTML

<div id="go-btn-group" class="btn-group">
    <a class="btn dropdown-toggle btn-success" data-toggle="dropdown" href="#">
        <img src="../../img/search.png" alt="Search" />
    <span class="caret"></span>
    </a>
    <ul id="btGo-dropdown" class="dropdown-menu">
        <li><a href="#" id="btUsername">Username</a></li>
        <li><a href="#" id="btLastName">Last Name</a></li>
        <li><a href="#" id="btStudentID">Student ID</a></li>
    </ul>
</div>

JS

    // All of the list items in the drop down 
    var $searchOptions = $('#btGo-dropdown li');
    $searchOptions.click(function (e) {
        var searchBy = '';

        // find all of the child links, which are
        // the options themselves
        var $lis = $searchOptions.find('a');

        // remove the active class, if exists
        $lis.filter('.active').removeClass('active');

        // add the active class to show the criteria selected
        var clickedOptionId = e.target.id;
        $('#' + clickedOptionId).addClass('active');

        // depending on the option selected, populate
        // searchBy with the URL to make the ajax call to
        switch (clickedOptionId) {
            case "btUsername":
                searchBy = 'SearchByUsername';
                break;
            case "btLastName":
                searchBy = 'SearchByLastName';
                break;
            case "btStudentID":
                searchBy = 'SearchByStudentID';
                break;
            default:
        }


        // create an object that abstracts the search data
        var search = {
            url: searchBy,
            data: { criteria: $search.attr('value') }
        };

        // make the call to the controller and get the raw Json
        var itemModels = $.ajax({
            url: search.url,
            type: "GET",
            contentType: "application/json; charset=utf-8",
            dataType: "json",
            data: search.data,
            async: false
        }).responseText;

        // Use knockout.js to bind the information to the page
        viewModel.rebindSearchItems(itemModels);

    });
\$\endgroup\$
2
\$\begingroup\$

I don't really think putting the URL in your HTML is a good idea; Separating presentation from functionality and all that...

However, instead of using a switch statement, you should be using an object literal to map the URLs to the clickedOptionId key:

var searchURLs = {
    'btUsername'    : 'SearchByUsername',
    'btLastName'    : 'SearchByLastName',
    'btStudentID'   : 'SearchByStudentID'
};

var search = {
    url: searchURLs[clickedOptionId],
    data: { criteria: $search.attr('value') }
};
\$\endgroup\$
  • \$\begingroup\$ Ah, I like that. The reason why I suggested the URL in the HTML is because that's what a lot of people do for autocompletes. I have seen data-autocomplete-url attributes used quite a bit. \$\endgroup\$ – David East Jul 19 '12 at 18:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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