My background is with C and python, and Javascript is fairly new to me, so I'm wondering if this is reasonable code.In particular, is it appropriate to use the higher order function makecb in this context, or is there a better/shorter/clearer way of writing it?$(function(){  var controllers = {    #showhelp: { name: Help, id: #help },    #showlog: { name: Log, id: #log }  };  function hideModals(){    for(cter in controllers){      $(controllers[cter].id).css(display, none);      $(cter).text(Show +controllers[cter].name);    }  }  function showModal(cter){    hideModals();    $(controllers[cter].id).css(display,block);    $(cter).text(Hide +controllers[cter].name);  }  function makecb(controller){    return function(){      if($(controllers[controller].id).css(display)=='none'){        showModal(controller);      }else{        hideModals();      }    }  }  for(cter in controllers){    $(cter).click(makecb(cter));  }});The idea is that the only thing that has to be done to add a new modal dialogue is to add the description. The appropriate html is as follows:<div id=bottombar>  <a id=showlog href=#>Show Log</a>   <a id=showhelp href=#>Show Help</a> </div><div id=log style=display:none></div><div id=help style=display:none></div>PS: If you have the privileges needed to add the tags data-driven-programming or idiom and you feel it would be appropriate, I'd appreciate it.
Is there a better way of writing this JavaScript, or is it idiomatic?
javascript;jquery
General tips:For calls done more than once and return the same results, cache them in a variableLook out for variable declarations. Always use var unless they are known to be somewhere in the outer scope. Otherwise, forgetting it would make it an implied global - which is bad.Localize variables whenever necessary. This is to prevent global conflict from destroying your references.Hard-coding is bad enough. Hard-coding in two or more places is worse. In your code, you are hard coding the ids to for modals, the links, and on the JS to link them both. Better use existing references and modify it to fit other references. In this case, use the href hashes. They were built to point to ids.DRY (Don't Repeat Yourself). Factor out common operations and turn them into their own functions. For example, the link text replace operation.In jQuery, chain when necessary, especially when the previous operation returns the elements needed for the next operation. This avoids multiple calls to jQuery just to re-reference the element for the next operation.Here's a working fiddleHere's a modified HTML<div id=bottombar>    <!--semantics and progressive enhancement tip: use hash to point to id'ed sections-->    <a href=#log>Show Log</a>    <a href=#help>Show Help</a> </div><!--HTML sections should be id'ed so hash links can jump to them if JS is off--><!--Use classes to refer to element of common functionality, both in CSS and JS--><div id=log class=modal></div><div id=help class=modal></div>CSS:/*use external style instead of inline*/.modal{    display:none;}here's a packed version (no comments):$(function ($) {    var modals = $('.modal');    function setText(prependText, oldText) {        return prependText + ' ' + oldText.substring(oldText.indexOf(' ') + 1)    }    $('#bottombar').on('click', 'a', function (event) {        event.preventDefault();        var $this = $(this),            modalid = $this.attr('href'),            modal = modals.filter(modalid);        if (modal.is(':visible')) {            modal.hide();            $this.text(function (index, oldText) {                return setText('Show', oldText)            })        } else {            modals.hide();            modal.show();            $this.text(function (index, oldText) {                return setText('Hide', oldText)            }).siblings().text(function (index, oldText) {                return setText('Show', oldText)            })        }    })});here's a commented version://use the provided local jQuery and don't rely on the global.//That way, name conflicts won't affect your inner code$(function ($) {    //get all modals and reference them    var modals = $('.modal');    //factor out the link text replace operation    //avoid regex on minor operations as it is slow    function setText(prependText, oldText) {        return prependText + ' ' +oldText.substring(oldText.indexOf(' ')+ 1);    }    //use delegation to add your handler. This avoids putting handlers on each link    $('#bottombar').on('click', 'a', function (event) {        //prevent the default action. Although hashes work, this one prevents        //the page from jumping upwards        event.preventDefault();        //cache items used over and over again in the following operations        var $this = $(this),            modalid = $this.attr('href'),            modal = modals.filter(modalid);        if (modal.is(':visible')) {            //if modal is visible            //hide the current modal            modal.hide();            //replace the current link text            $this.text(function (index, oldText) {                return setText('Show', oldText);            });        } else {            //if modal is not visible            //hide other modals            modals.hide();            //show the current modal            modal.show();            //set link texts accordingly            $this                .text(function (index, oldText) {                    return setText('Hide', oldText);                })                .siblings()                .text(function (index, oldText) {                    return setText('Show', oldText);                });        }    });});