trjhtr

I am working on a JavaScript exercise. I am running into issues with arrays.

NOTE: I posted this code in this forum because the code works. But, I would appreciate people reviewing it, and letting me know there are better ways to do this.

I am creating two arrays.

I am putting items into the first array. These are the separate parts of a string that represent a time: hours:minutes:seconds:milliseconds. You will see the times in the input string in my code.

I am putting those time segments into the first array, and then I am pushing that array into the second array (muti dimensional). I am doing this because I don't know how many times will be in an input string. I am also doing this because I need to eventually add up these times in the input string, and then add another set of time segments from a second input string that I did not include here in this code. (I realize that I have to convert these to numbers eventually.)

I tried the code below without the commented out line:

// arrSubtitlesSeparate = ;

I don't get what I expected.
When I include this line of code, I get the multi dimensional array I want.

Here is the results of the console logs. You will see them if you run the code.

You can see that the times are being found with the regex. But, the multi dimensional array is being filled with the last set of time segments that is found. You can easily see this just by looking at the last three digits which are the milliseconds in the time strings that are found.

found time: 00:13:37,243

arrSubtitlesSepMultiDim: 00,13,37,243

found time: 00:13:39,744

arrSubtitlesSepMultiDim: 00,13,39,744,00,13,39,744

found time: 00:13:39,779

arrSubtitlesSepMultiDim: 00,13,39,779,00,13,39,779,00,13,39,779

found time: 00:13:43,548

arrSubtitlesSepMultiDim: 00,13,43,548,00,13,43,548,00,13,43,548,00,13,43,548

If I add/uncomment that line of code, I get what I want.
The times are all added to the multi dimensional array.

My questions about coding this:

What is the proper way of creating the multidimensional array in this case?

Why is this happening when I don't include that line of code?

Does this have something to do with the fact that the first set of time segment values found in the input string are the first set of values being put into the array?

Would I be better off initializing the sub array with the first set of values, and then using the splice method to replace the values?

Here is my code. You should be able to copy/paste into an HTML doc.

 <script>

var subtitles = "188 00:13:37,243 --> 00:13:39,744 30 minutes. Everyone's ready. 189 00:13:39,779 --> 00:13:43,548 ♪ ";
var arrSubtitles = subtitles.split(" ");

console.log("arrSubtitles: " + arrSubtitles);

var arrSubtitlesSeparate = ;
var arrSubtitlesSepMultiDim = ;

for(var i = 0; i < arrSubtitles.length; i++)
 if(/[0-9]2:[0-5][0-9]:[0-5][0-9],[0-9]3/.test(arrSubtitles[i]) === true)
 console.log("found time: " + arrSubtitles[i]);
 arrSubtitlesSeparate[0] = arrSubtitles[i].charAt(0) + arrSubtitles[i].charAt(1);
 arrSubtitlesSeparate[1] = arrSubtitles[i].charAt(3) + arrSubtitles[i].charAt(4);
 arrSubtitlesSeparate[2] = arrSubtitles[i].charAt(6) + arrSubtitles[i].charAt(7);
 arrSubtitlesSeparate[3] = arrSubtitles[i].charAt(9) + arrSubtitles[i].charAt(10) + arrSubtitles[i].charAt(11);
 arrSubtitlesSepMultiDim.push(arrSubtitlesSeparate);
 // arrSubtitlesSeparate = ;
 console.log("arrSubtitlesSepMultiDim: " + arrSubtitlesSepMultiDim);
 


</script>

First of all, when presenting a piece of code to fellow developers, you help them the most by letting them know what your code does and what the inputs and expected outputs are. You immediately start by describing how your code does something, which is a bit confusing.

Regarding your code, I suggest the following:

• Both arrSubtitles and subtitles are arrays, but only one has the 'arr' prefix which is confusing. I find it more helpful to name variables according to their role instead of their type. You are splitting subtitles into some arbitrary whitespace delimited tokens, so how about tokens instead of arrSubtitles?




• arrSubtitlesSeparate and arrSubtitlesSepMultiDim are bulky, nondescript names. Following above advice, how about time and times?




• There is no need to perform a strict boolean comparison === true within your if-clause, your code looks neater by simply omitting that.




• Within the loop body, you access arrSubtitles[i] 10 times. To avoid those many lookups and improve readability, you could assign that array element to a temporary variable. Or you use a for ... of loop instead, which iterates over array values instead of array indices.




• Replace successive string.charAt concatenations with a single call to string.substr.




• Leverage the block scope of const and let instead of using the function scoped var.

Applying those suggestions to your code would result in e.g.

const regex = /[0-9]2:[0-5][0-9]:[0-5][0-9],[0-9]3/;
const times = ;

for (const token of subtitles.split(" ")) 
 if (regex.test(token)) 
 const time = [
 token.substr(0, 2),
 token.substr(3, 2),
 token.substr(6, 2),
 token.substr(9, 3)
 ];
 times.push(time);
 

console.log(times);

You asked what happens when you don't initialize arrSubtitlesSeparate = in the loop body?

Initially you create a single array instance arrSubtitlesSeparate. Then, during each loop iteration, you modify the content of this array and push it onto arrSubtitlesSepMultiDim. So in the end, each index of arrSubtitlesSepMultiDim contains a reference to the same arrSubtitlesSeparate array, similar to this:

arrSubtitlesSepMultiDim = [arrSubtitlesSeparate, arrSubtitlesSeparate, ...];

See also Is JavaScript a pass-by-reference or pass-by-value language?

Now, if we look again at your implementation, we note that you already perform a regex test. We could perform a regex matching instead and get rid of all those redundant string.substr calls. I wrapped this new implementation in a documented function and ended up with:

/**
 * Find and parse all timestamps of type "hours:minutes:seconds,milliseconds"
 * within a given subtitles string.
 * @param string subtitles
 * @return Array Array of times [hours, minutes, seconds, milliseconds]
 */
function findTimes(subtitles) 
 const regex = /([0-9]2):([0-5][0-9]):([0-5][0-9]),([0-9]3)/g;
 const times = ;

 let matches;
 while (matches = regex.exec(subtitles)) 
 times.push(matches.slice(1));
 
 return times;