I want to write better jQuery code

Question

I'm relatively new to jQuery and I while I'm trying to learn, I have this worry that I'm doing things... well not wrong... but not necessarily the right way.

I have the script below which works fine and does what I need it to do. But looking at it, I'm not satisfied, is it really the best way of achieving my goal or am I being too picky?

Brief description of what I'm building: A search form containing dynamically generated field. Each field has 2 classes to specify whether or not it should appear in the quick and/or advanced search form. Two links to toggle between the two views.

function searchModes() {
    $('p.quicksearch-true.advancedsearch-true').show();
    $('p.quicksearch-true.advancedsearch-false').show();
    $('p.quicksearch-false.advancedsearch-true').hide();
    $('p.quicksearch-false.advancedsearch-false').hide();
    $('#advanced').show();
    $('#quick').hide();

    $('#advanced').click(function () {
        $('p.quicksearch-true.advancedsearch-true').show();
        $('p.quicksearch-true.advancedsearch-false').hide();
        $('p.quicksearch-false.advancedsearch-true').show();
        $('#advanced').toggle();
        $('#quick').toggle();
        return false;
    });
    $('#quick').click(function () {
        $('p.quicksearch-true.advancedsearch-true').show();
        $('p.quicksearch-true.advancedsearch-false').show();
        $('p.quicksearch-false.advancedsearch-true').hide();
        $('#advanced').toggle();
        $('#quick').toggle();
        return false;
    });
}

The HTML is as follows:

<a href="#" id="quick">quick</a><a href="#" id="advanced">advanced</a>
<p class="quicksearch-true advancedsearch-true">
  Some dynamically generated form field that appears in both quick and advanced search
</p>
<p class="quicksearch-true advancedsearch-false">
  Some dynamically generated form field that appears in quick search only
</p>
<p class="quicksearch-false advancedsearch-true">
  Some dynamically generated form field that appears in advanced search only
</p>
<p class="quicksearch-false advancedsearch-false">
  Should never occur, but if it did, it would remain hidden anyway
</p>

So really, I'm after any feedback that you may have that could help me to try to write Better jQuery.

Much appreciated!

Answer 1

HTML:

<a href="#" id="quick" class="button" data-fields="q-field">Show Quick Form</a> <a href="#" id="advanced" class="button" data-fields="a-field">Show Advanced Form</a>

<p class="field q-field a-field">
  Some dynamically generated form field that appears in both quick and advanced search
</p>
<p class="field q-field">
  Some dynamically generated form field that appears in quick search only
</p>
<p class="field a-field">
  Some dynamically generated form field that appears in advanced search only
</p>
<p class="field">
  Should never occur, but if it did, it would remain hidden anyway
</p>

Simplified JQuery:

$(function() {
    $('#advanced').show();

    $('.button').click(function(e){
        e.preventDefault();
        $('.button').toggle('show');
        $('.field').hide().parent().find('p.' + $(this).data('fields')).show();
    });
});

http://jsfiddle.net/AlienWebguy/jsGmD/

Answer 2

In short, your code and markup is far too bloated for what you're trying to do.

I would scale back the markup to more like the following where

a) The shared fields are always onscreen and dont get hidden/shown b) The case which should never occur is removed - its irrelevant to the problem c) You don't have true/false versions of the classes - they are boolean; either there or not

<a href="#" id="quick">quick</a><a href="#" id="advanced">advanced</a>
<p>
  Some dynamically generated form field that appears in both quick and advanced search
</p>
<p class="quicksearch">
  Some dynamically generated form field that appears in quick search only
</p>
<p class="advancedsearch">
  Some dynamically generated form field that appears in advanced search only
</p>

Then your jQuery becoomes much much cleaner

$('#quick').toggle();
$('.advancedsearch').hide();

$('#advanced').click(function () {
     $('p.quicksearch').hide();
     $('p.advancedsearch').show();
     $('#advanced').toggle();
     $('#quick').toggle();
});
$('#quick').click(function () {
    $('p.quicksearch').show();
    $('p.advancedsearch').hide();
    $('#advanced').toggle();
    $('#quick').toggle();
});

Live example here: http://jsfiddle.net/6nkXN/