Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.

What I want to happend with the following code is when a user checks multiple data centers and then selects a type of change it will automaticly refresh the description and Impact text area with a unique string statement including the data centers the user has choosen.

can someone advice me what I am doing wrong?

JS:

function updateDescImpact() {
    var changeSel = document.changeform.change_type;
    var ChangeType = (changeSel.options[changeSel.selectedIndex].value);
    var description = " ";
    var impact = " ";
    var data_center = "";
    var inputs = document.getElementsByTagName('input');
    for (var x = 0; x < inputs.length; x++) {
        {
            if (inputs[x].type == "checkbox" && inputs[x].name == 'data_center[]') {
                if (inputs[x].checked == true) {
                    data_center += inputs[x].value + ',';
                }
            }
        }
        if (/,$/.test(data_center)) {
            data_center = date_center.replace(/,$/, "")
        }
        if (ChangeType == "Filer Changes") {
            description = "This is the Filer Change Description for $('data_center')";
            impact = "This is the Filer Changes impact statement for $('data_center')";
        } else if (ChangeType == "DNS Changes") {
            description = "This is the DNS Change Description for $('data_center')";
            impact = "This is the DNS Changes impact statement for $('data_center')";
        } else {
            description = "";
            impact = "";
        }
        document.changeform.description.value = description;
        document.changeform.impact.value = impact;
    }

HTML:

<form action="" id="changeform" method="post" name="changeform">
    <input type="submit" value="submit change">
    <table>
        <tr valign="top">
            <td><strong>Data Center</strong></td>
            <td><input name="data_center[]" type="checkbox" value="zone1">Zone1
            <input name="data_center[]" type="checkbox" value=
            "Zone2">Zone2</td>
        </tr>
        <tr valign="top">
            <td><strong>Change Type</strong></td>
            <td><select id="change_type" name="change_type" onchange=
            "updateDescImpact()">
                <option value="DNS Changes">
                    DNS Changes
                </option>
                <option value="Filer Changes">
                    Filer Changes
                </option>
            </select></td>
        </tr>
        <tr valign="top">
            <td><strong>Description</strong></td>
            <td>
            <textarea cols="50" id="description" name="description" rows="10">
This text needs to be updated
    </textarea>
            </td>
        </tr>
      <tr valign="top">
            <td><strong>Service Impact</strong></td>
            <td>
            <textarea cols="50" id="impact" name="impact" rows="10">
This text needes to be updated
</textarea></td>
        </tr>
    </table>
</form>
share|improve this question
 
Do you get any errors? –  putvande Jul 10 at 17:45
 
I did not get any errors, but the description and Impact text is not being refreshed with the statements and data center –  Bryce Turng Jul 10 at 17:59
 
This $('data_center') makes it seem like you're using jQuery, or have it available. Is that an option for you (it's not a requirement by any means)? –  David Thomas Jul 10 at 18:03
 
thanks David, if the format is incorrect, can you advice me how I would display an array of data centers within the string javascript string? –  Bryce Turng Jul 10 at 18:05
 
Sure, where do you want to put it in the string? In which string? Do you want to replace the $('.data_center') string with the checkbox values? –  David Thomas Jul 10 at 18:12
show 2 more comments

1 Answer

up vote 0 down vote accepted

With only a minimal idea of what it is you want to do with the string, once you've got it, I've put together the following:

function updateDescImpact() {
    // declaring all the variables, separating them with commas, at once.
    // retrieves the element:
    var changeSel = document.getElementById('change_type'),
        // defines the various default messages:
        changes = {
            'DNS': {
                'impact': 'This is the DNS Change impact statement for $("data_centre")',
                'description': 'This is the DNS Change Description for $("data_center")'
            },
                'Filer': {
                'impact': 'This is the Filer Changes impact statement for $("data_centre")',
                    'description': 'This is the Filer Change Description for $("data_center")'
            }
        },
        ChangeType = (changeSel.options[changeSel.selectedIndex].value),
        // creates an array:
        data_center = [],
        inputs = document.getElementsByTagName('input');

    for (var x = 0, len = inputs.length; x < len; x++) {
        {
            if (inputs[x].type == "checkbox" && inputs[x].name == 'data_center[]') {
                if (inputs[x].checked === true) {
                    // adds the checkbox-values to the array:
                    data_center.push(inputs[x].value);
                }
            }
        }
        // creates another key for the 'changes' object,
            // the value of which is determined by whether the array contains values,
            // if it *does*, then those values are joined together using the given string,
            // if *not*, then the value is an empty string.
        // in both cases the value is followed by a period.
        changes.data_centers = (data_center.length > 0 ? ': ' + data_center.join(', ') : '') + '.';
        document.getElementById('description').value = changes[ChangeType].description + changes.data_centers;
        document.getElementById('impact').value = changes[ChangeType].impact + changes.data_centers;
    }
}

JS Fiddle demo.

In the next part I'll try to explain the changes I've made, but the simple, central, premise is to minimise the amount of work you're doing, and to reduce the scope for errors to creep in.

Where you were using:

document.changeform.change_type;

I've chosen, instead, to use:

var changeSel = document.getElementById('change_type');

This is because using the id of the relevant element is faster than navigation through the DOM using two ids, especially since there's no guarantee that the element will be recognised by its id when it's used as a global variable without being declared. It works almost everywhere, yes, but it's not a standard, or specified, behaviour in JavaScript and was a convenience offered, initially, in Internet Explorer. This convenience is not guaranteed to present in future, however.

Also, an id is (and must be) unique within the document, therefore there's no sense in using two, when one will do perfectly well.

The use of an object to contain the various alternative strings you (might) want to use, this is to reduce the need for multiple if/else if/else statements. And makes it somewhat easier to update the function in future to address new options and requirements. As stated, above, this is to minimise the amount of work you have to do (now, and in the future).

Where you used:

if (/,$/.test(data_center)) {
    data_center = date_center.replace(/,$/, "")
}

I chose, instead, to use an array:

data_center = [],

This allows us to avoid having to use regular expressions which, while in this case was simple enough, is not always the case and, again, is more difficult than necessary for the finished result. Whereas an array can use the .join() method (used later) which will join the array elements together with the defined string, and will not result in a trailing-comma.

I've also used getElementById() in the last two lines to assign the various strings/values to the relevant elements. For precisely the same reason as outlined above.

share|improve this answer
 
the new code you provided worked like a charm.. expect I removed the $("data_centre") from the statement... Now I waiting for your details on what the updates does? thanks you very much –  Bryce Turng Jul 10 at 18:55
 
You're very welcome indeed, the reason I left in the $("data_centre") was simply because I didn't know where you wanted the values to go (hence my question(s) in the comments, above. I assumed it was a placeholder, but without confirmation I didn't want to change things too much. –  David Thomas Jul 10 at 18:57
 
I am average Perl cgi coder and started to learn JavaScript... I would have been able to resolve this issue with Perl, but needed assist with JavaScript. In your opinion which scripting language is better for this project JavaScript or Perl? In terms of Dynamic web pages and events handling? –  Bryce Turng Jul 10 at 19:05
 
I honestly don't know enough of Perl to offer an objective opinion. For the web, I always prefer JavaScript; but I'm clearly biased. I'd suggest looking at what you're trying to achieve, and looking at the anticipated difficulties you might have with Perl, and the same for JavaScript. Then pick whichever tool is likely to give you the fewest insurmountable problems. –  David Thomas Jul 10 at 19:12
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.