2

I am new to JavaScript and am learning how to store values into a custom object. I am creating a program that will loop through a series of check boxes and once store the values of the checked boxes into an object.

I've stripped down my code to the most simple version

which can be viewed on JSfiddle here: https://jsfiddle.net/Buleria28/tyapjno2/1/

The HTML is:

<form>
  <div>
    <label>
      <input type="checkbox" name="style" value="classical" 
       onchange="add()">Classical</label>
    <label>
      <input type="checkbox" name="style" value="Rock" 
       onchange="add()">Rock</label>
    <label>
     <input type="checkbox" name="style" value="Jazz" 
     onchange="add()">Jazz</label>
    <label>
     <input type="checkbox" name="style" value="Folk" 
     onchange="add()">Folk</label>
  </div>
</form>

The JavaScript is:

var interest = {
    styles: "",
};

function add(){
    var sbox = document.getElementsByName("style");
    for (var i=0; i<sbox.length; i++){
    if (sbox[i].checked == true){
    interest.styles = sbox.value;   
        }
    }
 }

When I try to view the "interest" object in chrome developer mode it says that styles is undefined. Is there an issue with my looping through the check boxes or an issue with how I am storing the value of the checked boxes into the object?

user3063771
  • 75
  • 2
  • 8
  • 1
    It should be `interest.styles = sbox[i].value`. Keep in mind you will only get the value of one checked checkbox the way you have it. If you want to get multiple values you can use an array or add to the empty string in `interest.styles` using `+=` instead of just `=`. – DonovanM Jun 18 '17 at 01:25

2 Answers2

4

Recording data

It's important to note that there are many ways to record/store data, and each way may be ideal for one situation, but terrible for another. It entirely depends on how you want to use the stored data, as to how best to store it.

Your question

Is there an issue with my looping through the check boxes or an issue with how I am storing the value of the checked boxes into the object?

There are also many ways to loop through data, and again, each way has its merits and faults. Your loop works and isn't bad code but (from MDN docs about getElementsByName):

The name attribute is only applicable to (X)HTML documents. The method returns a live NodeList Collection that contains all elements with a given value for the name attribute, such as <meta> or <object> or even if name is placed on elements which do not support a name attribute at all.

Working with a "live NodeList" can sometimes have unexpected results when iterating over them. Converting the NodeList to an Array by use of .from() gets around those possible hiccoughs.

Better still would be to simply use querySelectorAll( "input[name=style]" )

Where you went wrong

The line:

interest.styles = sbox.value;

states that the string value of "interest"s property "styles" is equal to the value of the NodeList, and not to the value of the <input> element. The NodeList has no value, so the value is always undefined.

interest.styles = sbox[ i ].value;

will solve the first problem, but then you're only ever storing the last checked value. If there are 3 checkboxes checked, only one will be remembered.

interest.styles += sbox[ i ].value;

will store the values of all the check checkboxes, but the string will be a mess. Some formatting could be added to improve it, but there are simpler and more useful methods.

Adding "styles" to an array

It will depend on what you want to do with your "styles" once you've got them, but this creates an array of all the selected "styles".

Please request more details if needed.

var interest = {
    "styles": [] // use an array to store the styles
};

function add(){
    var sbox = Array.from( document.getElementsByName( "style" ) );
    interest.styles = []; // empty the array before rebuilding it
    sbox.forEach( function( v ) {
        if ( v.checked ) {
            interest.styles.push( v.value );   
        }
    } );
    // console output for demo
    console.log( interest.styles ); // array
    console.log( interest.styles.join() ); // CSV
    console.log( interest.styles.join( " " ) ); // for HTML class attributes
}
<form>
  <div>
    <label>
      <input type="checkbox" name="style" value="classical" 
       onchange="add()">Classical</label>
    <label>
      <input type="checkbox" name="style" value="Rock" 
       onchange="add()">Rock</label>
    <label>
     <input type="checkbox" name="style" value="Jazz" 
     onchange="add()">Jazz</label>
    <label>
     <input type="checkbox" name="style" value="Folk" 
     onchange="add()">Folk</label>
  </div>
</form>

An optimization

Add the add() function to the <form> element, to process all form changees in one place. In this case, the function might be better called update().

var interest = {
    "styles": [] // use an array to store the styles
};

function update(){
    var sbox = Array.from( document.getElementsByName( "style" ) );
    interest.styles = []; // empty the array before rebuilding it
    sbox.forEach( function( v ) {
        if ( v.checked ) {
            interest.styles.push( v.value );   
        }
    } );
    // console output for demo
    console.log( interest.styles ); // array
    console.log( interest.styles.join() ); // CSV
    console.log( interest.styles.join( " " ) ); // for HTML class attributes
}
<form onchange="update()">
  <div>
    <label>
      <input type="checkbox" name="style" value="classical">Classical</label>
    <label>
      <input type="checkbox" name="style" value="Rock">Rock</label>
    <label>
     <input type="checkbox" name="style" value="Jazz">Jazz</label>
    <label>
     <input type="checkbox" name="style" value="Folk">Folk</label>
  </div>
</form>

Also, consider using addEventListener instead of inline events.

Or create an object

Creating an object with keys formed from the "styles" and boolean values equal to those of the respective .checked can be handy.

var interest = {
    "styles": {} // use an object to store the styles
};

function update(){
    var sbox = Array.from( document.getElementsByName( "style" ) );
    sbox.forEach( function( v ) {
        interest.styles[ v.value ] = v.checked; // e.g. interest.styles.Rock = false
    } );
    // console output for demo
    console.log( interest.styles.Rock ); // does this user like "Rock"?
}
<form onchange="update()">
  <div>
    <label>
      <input type="checkbox" name="style" value="classical">Classical</label>
    <label>
      <input type="checkbox" name="style" value="Rock">Rock</label>
    <label>
     <input type="checkbox" name="style" value="Jazz">Jazz</label>
    <label>
     <input type="checkbox" name="style" value="Folk">Folk</label>
  </div>
</form>
Fred Gandt
  • 4,217
  • 2
  • 33
  • 41
-1

as @Donovan said,, you should use sbox[i] reference in your loop,, also I would suggest to add array and push values there,, and also take care of empting it, or even better would be to only replace values on change..

var interest = {
    styles: [],
};

function add(){
    var sbox = document.getElementsByName("style");
    for (var i=0; i<sbox.length; i++){
      interest.styles.push( sbox[ i ].checked );   
    }
    console.log( interest.styles );
    interest.styles.length = 0;
 }
<form>
  <div>
    <label>
      <input type="checkbox" name="style" value="classical" 
       onchange="add()">Classical</label>
    <label>
      <input type="checkbox" name="style" value="Rock" 
       onchange="add()">Rock</label>
    <label>
     <input type="checkbox" name="style" value="Jazz" 
     onchange="add()">Jazz</label>
    <label>
     <input type="checkbox" name="style" value="Folk" 
     onchange="add()">Folk</label>
  </div>
</form>
Kresimir Pendic
  • 3,597
  • 1
  • 21
  • 28