Managing network address information

I'm a developer at a networking company that really has no peers that work above me that I can use for any sort of sounding board for my code, so it's just me. I was wondering if anyone would be willing to take a look at some of my code and just give me a few notes, bullet points, etc... on things that are wrong, things I should look into and so on. If anyone is interested please feel free to comment.

I'm concerned with the JavaScript/jQuery. It seems very coupled and I was wondering if there are any better ways to accomplish the things that I'm doing with this code.

Everything works, but I'm always sure there is room for improvement.

//Global networkInfo Object.  
var networkInfo = {};

$(function () {

    // Bind custom events

    $(networkInfo).bind('updateNetworkInfo', updateNetworkAddress);
    $(networkInfo).bind('updateNetworkRanges', updateNetworkRangeDropDowns);
    $(networkInfo).bind('selectNetworkRanges', selectNetworkRanges);

    // Set form validation

    $('form').validate({
        messages: {
            tbSiteName: "This is an invalid site code"
        }
    });

Add custom rule for site name. This rule has been a pain because this is a webforms application and the name of the input can't be controlled.

$('#tbSiteName').rules("add", {
        required: true,
        remote: function () {
            var r = {
                url: "/webservices/ipmws.asmx/SiteValid",
                type: "POST",
                data: "{'tbSiteName': '" + $('#tbSiteName').val() + "'}",
                dataType: "json",
                contentType: "application/json; charset=utf-8",
                dataFilter: function (data) { return (JSON.parse(data)).d; }
            }
            return r;
        }
    });

DrillDownProvisioning.aspx doesn't allow the user to change the Subnet mask. They should only be allowed to change this value by clicking on nodes in the DrillDown Tree.

```
$('#ddSubnetMask').change(function (e) {

From perusing your code a few times:

Custom event abuse

While it's very cool to have custom events, I would simply remove this extra layer of logic. It does not make sense to call $(networkInfo).trigger('selectNetworkRanges'); if you could just call selectNetworkRanges(). I understand that you would loose access to this but you are accessing networkInfo directly in updateNetworkRangeDropDowns anyway.

DRY (Don't Repeat Yourself)

-
In selectNetworkRanges you could do var ranges = this.networkRanges; and then access ranges instead of this.networkRanges every time

-
You are building a dropdown in populateSubnetMask and in updateNetworkRangeDropDowns in a different way even though the functionality is very close. With some deep thoughts you could create a helper function that could build a dropdown for both #ddSubnetMask and .mask

-
$('.open').click and $('.close').click do the same really, you could just do this:

$('.close,.open').click(function () {
    $('#drilldowntreecontainer').toggle('1000');
    $('.open').toggle();
});

What's in a name?

-
Please avoid short names like r , d

-
It is considered good practice to prefix jQuery results with $ so var $dropDown = $('#ddSubnetMask'); for example

Style

-
Comma separated variables with a single var are considered better so

var node = eventArgs.get_node(),
    address = node.get_value().split("/");

instead of

var node = eventArgs.get_node();
var address = node.get_value().split("/");

-
You are using both double quotes and single quotes for your string constants, you should stick single quote string constants. With the possible exception of your data: statements.

Comments

-
Great commenting in general, maybe a tad too verbose at times

-
You should mention in the top that this code relies on the jQuery Validation Plugin, in fact it would have saved time if you mentioned that in your question ;)

Design

-
ddSubnetMask could be set as disabled, you would need a hidden input that contains the actual value to be submitted as per https://stackoverflow.com/a/368834/7602

-
The last few functions starting with populateSubnetMask are not within your $(function () {, I would keep it all together

Magic Numbers

-
You are commenting that VLAN(type=9), I would still advocate to create a var IS_VLAN = 9 and then use that constant

-
Not a magic number per se, 'application/json; charset=utf-8' should be a properly named constant ( it's a DRY issue as well ).

Dancing in the rain

• Your $.ajax calls should deal with error, it will happen at some point

All in all, I could work with this code. You are correct that the code is tightly coupled. I think that's because of the data you have to work with, so I would not worry about it too much.

StackExchange Code Review Q#15182, answer score: 20