1
\$\begingroup\$

I'm new with JavaScript and need your advice. The idea is to show/hide text field depending on select option. I have a working code, but I believe it could be more readable and better. What have I missed?

   <script type="text/javascript">
<!--
    function showHide(){
        var selectedValue =document.getElementById("choice_criteria_id").value;
        var src_addr = document.getElementById("id_src_addr");
        var dst_addr = document.getElementById("id_dst_addr");
        var port = document.getElementById("id_port");
        var port_range = document.getElementById("id_port_range");

        if (selectedValue == "source_ip"){
            src_addr.style.display = "inline";
            dst_addr.style.display = "none";
            port.style.display = "none";
            port_range.style.display = "none";


        } else if (selectedValue == "dest_ip"){
            dst_addr.style.display = "inline";
            src_addr.style.display = "none";
            port.style.display = "none";
            port_range.style.display = "none";


        } else if (selectedValue == "port"){
            port.style.display = "inline";
            src_addr.style.display = "none";
            dst_addr.style.display = "none";
            port_range.style.display = "none";

        } else if (selectedValue == "port_range"){
            port_range.style.display = "inline";
            src_addr.style.display = "none";
            dst_addr.style.display = "none";
            port.style.display = "none";
        }
}

-->
</script>
\$\endgroup\$
1
  • 1
    \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$
    – BCdotWEB
    Mar 23 '16 at 13:58
3
\$\begingroup\$

You can use Conditional (ternary) Operator.

Syntax :

condition ? expr1 : expr2 

Reformated code :

function showHide(){
    var selectedValue =document.getElementById("choice_criteria_id").value;
    document.getElementById("id_src_addr").style.display = (selectedValue == "source_ip") ? "inline" : "none";
    document.getElementById("id_dst_addr").style.display = (selectedValue == "dest_ip") ? "inline" : "none";
    document.getElementById("id_port").style.display = (selectedValue == "port") ? "inline" : "none";
    document.getElementById("id_port_range").style.display = (selectedValue == "port_range") ? "inline" : "none";
}

Example in CodePen

\$\endgroup\$
0
0
\$\begingroup\$

Use custom "map" object on the following scheme: {<element_id> : <dependent_value>}.
You can easily extend such object in case if there would be much more elements:

function showHide(){
    var map = {"id_src_addr":"source_ip", "id_dst_addr":"dest_ip", "id_port":"port", "id_port_range":"port_range"}, 
        selectedValue = document.getElementById("choice_criteria_id").value;

    Object.keys(map).forEach(function(id){
        document.getElementById(id).style.display = (selectedValue === map[id]) ? "inline" : "none";
    });
}

That's all ... )

\$\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.