I use a lot of when user clicks on element then another element will be displayed as visible/hidden... aka toggle. So I'm trying to make a general function to abide the DRY principal.Before jumping into the code, I just wanted to state that there are still bugs. I just want to know before continuing if writing a function like the one below is necessary or if I should just ignore the DRY in this case and have lots of separate toggle functions. Or if it's necessary, would there be a more efficient method? JSFiddle*Brief Description of Code -There are 4 parametersThe first two are mandatory, the last two are optionalThe index parameter is a string of numbers ex. 1.0,1The reason I need the last two parameters is because I have elements that can't be easilytargeted. For example, they don't have an #id. So to grab those generic elements, I use getElementsByTagNameThen I'll use the index parameter to get the element in the array.index parameter: The number before '.' is the index of the clickElementand the number after '.' is the index of the toggleElement. clickElement = 1. toggleElement = 0. function toggleClick(clickElement, toggleElement, condition, index) {     var toggleIndex;    var clickIndex;    if(condition === false){        if(index.indexOf('.') != -1){            //indexOf returns -1 if '.' is not found.             // Checks to see if two index is entered            indexArray = index.split('.');            clickIndex = indexArray[0];            toggleIndex = indexArray[1];            clickElement = document.getElementsByTagName(clickElement)[clickIndex];            toggleElement = document.getElementsByTagName(toggleElement)[toggleIndex];        }else{            //If there is no '.', that means only one index was entered.            // By function requirement, it should be the index of clickElement            clickElement = document.getElementsByTagName(clickElement)[index];        }    }    $(clickElement).click(function(){            $(toggleElement).toggle();        });    } This is one of my first functions, and so I need some guidance on whether I'm writing bad code. Yes, this might be broad because there may be many different ways to write this function.  But I just need to know if there's a more efficient way or if my way is okay.
Creating a function when user clicks on element to toggle another element
javascript;jquery;event handling
I know you're trying to be flexible here, but your approach comes at a cost: The API isn't understandable. I can see you're trying to toggle the second parameter element with the first parameter as the toggle,  But I don't understand exactly how the third or fourth works, if it's even needed. It's also a weird convention.Reflect on these three things here:The point of programming is having stuff done easier. The point of a programming language is to do programming in that's understandable to humans. The point of an API is to lift (abstract) the programming language.Fail on one of those, and the rest before it breaks apart (and things like the great flood, alien invasions, world war 3 happen). So before you design an API, or even just a function, think about the users/developers using it. It might be you in the distant future.Now off to your code. Seeing you're using jQuery, a similar feat could have been done through this piece of code. I do notice you said elements that can't be easily targeted. For example, they don't have an #id. - jQuery still can target them using your standard CSS selectors (here's a few to remember), and some custom jQuery selectors (which I think is a superset of CSS selectors, if I'm not mistaken).. The :eq() part is optional, but it does what you need, select specific indices.// Assuming delegation for future elements$(document).on('click','li:eq(0)',function(){  $('div:eq(0)').toggle();});It's as simple as Get the first of all <li> and toggle the first of all <div>. Simple enough. But what if you wanted your own simple function for it and not some jQuery mashup that is copy-paste bait? Something like this could be done too:;(function ($) {  $.fn.customToggle = function (trigger, target, event) {    // Let's add a twist, custom functions. Assume click if not defined.    event = event || 'click';    this.on(event, trigger, function () {      $(target).toggle();    });  }}(jQuery));// Still, future-proof, let's support future elements$(document).customToggle('li:eq(0)', 'div:eq(0)');$(document).customToggle('li:eq(1)', 'div:eq(1)','mouseover');There you have it. Now you'd wonder That's not any different from my code - exactly. It's the same. However, there's one important bit I changed - it's the way I selected the elements and indices. I used CSS selectors - which every developer who came across CSS or jQuery should already know.Here are other suggestions on how you could structure the API:// Chained:$(document)  .customToggle('li:eq(0)', 'div:eq(0)')  .customToggle('li:eq(2)', 'div:eq(4)')  .customToggle('li:eq(3)', 'div:eq(9)')// Map$(document).customToggle({  'li:eq(0)' : 'div:eq(0)',  'li:eq(2)' : 'div:eq(4)',  'li:eq(3)' : 'div:eq(9)'});