How do I correctly split an array of strings of is it another issue?

Question

I am trying to access a random element from an array of strings as per other examples here on SO. I am using Raphael.js and regions[j] below returns an array of Raphael objects - hence the .data(id). This seems to be ok, but theCountyNames, as outlined in the comment below returns all of the strings as one long string. I am guessing that this is why randCounty returns a single random letter, but when I try appending a comma in the loop (+",") and using split as per this question, I still get one random single letter. Perhaps I am implementing this incorrectly or maybe it is another issue? Thanks.

 function pickRandCounty(){
var theCountyNames = new Array();
for (var j = 0; j < regions.length; j++) {
theCountyNames = regions[j].data('id');
document.write(theCountyNames);//THIS GIVES ME THE COMPLETE LIST OF ITEMS IN THE ARRAY BUT ALL AS ONE STRING
//document.write("<hr>");
 }
//var randCounty = theCountyNames[Math.floor(Math.random() * theCountyNames.length)];
//document.write(randCounty);//THIS JUST RETURNS ONE RANDOM LETTER??
}

You seem to be `var`ing `theCountyNames` twice. You seem to be overwriting the old value of `theCountyNames` in each iteration instead of appending. You are also using `document.write`? :( — Paul S., Aug 15 '15 at 17:11
Had just noticed the duplicate declaration and edited. The document.write comes from a tutorial I was following here https://www.youtube.com/watch?v=pqLS4oyJ8cA — Inkers, Aug 15 '15 at 17:12
It's much better practice to use _Console_ methods such as `console.log` to debug (or test), usually you can open the console with F12, otherwise see http://webmasters.stackexchange.com/questions/8525/how-to-open-the-javascript-console-in-different-browsers — Paul S., Aug 15 '15 at 17:22

score 2 · Accepted Answer · answered Aug 15 '15 at 17:14

2

Using Array.prototype.push to append a new item to an Array.

function pickRandCounty(){
    var theCountyNames = [],
        j;
    for (j = 0; j < regions.length; ++j) {
        theCountyNames.push(regions[j].data('id'));
    }
    j = Math.floor(Math.random() * regions.length);
    return theCountyNames[j];
}

However, this is not optimised as you can set the length of the Array in advance and you can even completely skip the loop,

function pickRandCounty(){
    var j = Math.floor(Math.random() * regions.length);
    return regions[j].data('id');
}

answered Aug 15 '15 at 17:14

Paul S.

64,864
9
122
138

The optimized part is great. I couldn't think in that direction. – Rudra Aug 15 '15 at 17:21
Sound, working now. Thanks. Will take me a while to work through optimised version to understand why it works that way!! – Inkers Aug 15 '15 at 17:26
@Rudra For these types of problems consider the order of operations and whether you can swap them, i.e. _applying a transformation (to an entire array) then selecting one_ **vs** _selecting one then applying the transformation (to the single one)_ – Paul S. Aug 15 '15 at 17:34
@Paul S Great suggestion. I would keep that in mind – Rudra Aug 15 '15 at 17:37

score 0 · Answer 2 · answered Aug 15 '15 at 17:15

0

The error seems to be in this part of line.

theCountyNames = regions[j].data('id'); //wrong
theCountyNames.push(regions[j].data('id')); //right

Second mistake

document.write(theCountyNames); //it will keep on appending the string in the DOM
document.write("<br>" + theCountyNames);//right

answered Aug 15 '15 at 17:15

Rudra

1,678
16
29

Thanks Rudra, accepted other answer as was first and added more to the solution, but thanks for taking time to help. – Inkers Aug 15 '15 at 17:27

How do I correctly split an array of strings of is it another issue?

2 Answers2