4
\$\begingroup\$

This is a pretty straightforward library function I've got here. I've had to build it in to about 3 different forms but I never heard of this kind of function anywhere else.

I'm trying to make it as readable and clear as possible, but I also am not too familiar with the best way to capture values in JavaScript. As far as I know, the following disableChecker function allows me to capture the value arguments at declaration time so I can have many of these DisableIf checks going simultaneously.

    function disableChecker(checkSelector, disableValue, targetSelector) {
        var optionValue = $(checkSelector).val();
        if (optionValue === disableValue) {
            $(targetSelector).prop("disabled", true); //Disable target
        } else {
            $(targetSelector).prop("disabled", false); //Enable target
        }
    }
    function DisableIf(checkSelector, disableValue, targetSelector) {
        $(checkSelector).on("change,keyup,blur", disableChecker(checkSelector, disableValue, targetSelector));
    }

Usage:

<select id="disableSource">
    <option value="0">Disable Next</option>
    <option value="1">Enable Next</option>
</select>
<input type="text" id="disableTarget" />
<input type="text" id="disableTarget2" />

<script type="text/javascript">
    //Please note that at this time, order is important!
    DisableIf("#disableSource", "0", "#disableTarget");
    DisableIf("#disableTarget", "Disable Next", "#disableTarget2");
    DisableIf("#disableSource", "0", "#disableTarget2");
<script>

jsFiddle

\$\endgroup\$
1
\$\begingroup\$

Readability:

Merging code into a single line:

  • Variables - If the function isn't too complex to understand, variable declarations that are only used in a single location are unnecessary.
  • If-statements - If-Statements take a boolean and do something different based on the boolean. If you're toggling a value like "disabled" you usually can just pass the boolean straight to the value-setter.

    function disableChecker(checkSelector, disableValue, targetSelector) {
         $(targetSelector).prop("disabled", $(checkSelector).val() === disableValue);
    }
    

Library:

  • Naming - It should be clear-er to anyone working with the code what each function does (whether this person be you, someone else, or you 6 months from now).
  • Condition - Why force it to always be an equality condition? Perhaps in the future you may want it to be greater-than or something even more complex? By passing in a boolean parameter instead you expand your library function's potential.
  • Maintainability - Single Responsibility Principal. Each function does one specific thing so that if any of the parts change they aren't all affected.
  • Initialization - This is the only slight down-side that I see. It takes a little bit more code turning the condition into a parameter. But flexibility and readability usually come at some kind of cost (generally in the setup or performance departments). Given that it's a library function though, you're probably wanting that flexibility.

Something like:

function setDisable(targetSelector, isDisabled) {
    $(targetSelector).prop("disabled", isDisabled);   
}

function valueEqualsExpected(selector, expectedValue) {
    return $(selector).val() === expectedValue;
}

function turnOnAndRunEvent(events, selector, func) {
    $(selector).on(events, func);
    //$(document).on(events, selector, func);
    func();
}

turnOnAndRunEvent("change keyup blur", "#disableSource", function () {
    setDisable("#disableTarget", valueEqualsExpected("#disableSource", "0"));
});
turnOnAndRunEvent("change keyup blur", "#disableTarget", function () {
    setDisable("#disableTarget2", valueEqualsExpected("#disableTarget", "Disable Next"));
});
turnOnAndRunEvent("change keyup blur", "#disableSource", function () {
    setDisable("#disableTarget2", valueEqualsExpected("#disableSource", "0"));
});

JSFiddle

\$\endgroup\$

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.