3
\$\begingroup\$

I've written the code below with what little knowledge I have of JavaScript at the moment.

The code simulates the process of removing multiple videos at once from a playlist like the one you see on YouTube. As it stands now, the code removes a total of four videos (each positioned at the index 0, 2, 5, and 7), using the combination of the for loop and splice methods. But here are a couple of things I can't figure out on my own:

1. Practically speaking, videos to be removed, and by extension, their corresponding indexes in an array should not be predetermined, but rather be selected by the user. In other words, the playlistIndex array should be empty at first, and index numbers should be added to it one by one as the user ticks checkboxes.

2. Even after I have ironed out the first problem mentioned above, the current code will remove a selected video immediately after one index number is pushed to the playlistIndex array. So I have to make it so that the code does not start to remove videos until the user has selected every video they want to have removed.

3. Lastly, before setting the removal process in motion, the code needs to have the user confirm their decision. Ideally, the user should be asked to press "yes" or "no" in a pop-up that goes something like "You are about to remove 7 videos. Are you sure you want to proceed?"


var playlist = ["video1", "video2", "video3", "video4", "video5", "video6", "video7", "video8", "video9", "video10"];

var playlistIndex = [0, 2, 5, 7];

var bin = [];


for (var i = playlistIndex.length - 1; i >= 0; i--) {
    bin.unshift(playlist.splice(playlistIndex[i], 1));
}


for (var j = bin.length -1; j >= 0; j--) {
    if (bin[j].length === 0) {
        bin.splice(j, 1);
    }
}


if (playlist.length === 0) {
    alert("No videos in the playlist.");
}
else {
    alert(playlist.length+" video(s) remain in the playlist: "+playlist.join(", ")+".");    
}


bin = [].concat.apply([], bin);

alert(bin.length+" video(s) have been removed: "+bin.join(", ")+".");
\$\endgroup\$
5
\$\begingroup\$
for (var i = playlistIndex.length - 1; i >= 0; i--)

Instead of having it negatively looping, which in this instance is unnecessary, do it positively:

for (var i = 0, length = playlistIndex.length; i < length; i++){

Instead of the negatively recurring for loop, consider using a length subtraction operation instead like in the example below:

for (var j = bin.length -1; j >= 0; j--) {
    if (bin[j].length === 0) {
        bin.splice(j, 1);
    }
}

into:

for (var i = 0, length = bin.length; i <= length; i++){
    bin.splice(length - 1 - i, 1);
}

For this example, a negative iteration is the way to go, because otherwise as @Tibos pointed out in the comments below, the array indexes can get messed up and end up leaving them in.

For example, if b were ['a', 'b', 'c'], a normal for loop here with bin.splice(i, 1) would leave 'b'.


bin.unshift with the a negative for loop is the same as bin.push with a positive for loop.

Positive for loops are always preferred when possible.

for (var i = playlistIndex.length - 1; i >= 0; i--) {
    bin.unshift(playlist.splice(playlistIndex[i], 1));
}

into:

for (var i = 0, length = playlistIndex.length; i < length; i++){
    bin.push(playlist.splice(playlistIndex[i], 1));
}

You should have whitespace between your operators:

playlist.length+" video(s) remain in the playlist: "+playlist.join(", ")+"."

into:

playlist.length + " video(s) remain in the playlist: " + playlist.join(", ") + "."

The following block can be converted to a ternary statement:

if (playlist.length === 0) {
    alert("No videos in the playlist.");
}
else {
    alert(playlist.length+" video(s) remain in the playlist: "+playlist.join(", ")+".");    
}

into:

alert(playlist.length === 0
    ? "No videos in the playlist"
    : playlist.length + " video(s) remain in the playlist: " + playlist.join(", ") + "."
);

bin = [].concat.apply([], bin): this seems unnecessary; I wouldn't bother doing it.

I thought this was unnecessary, but I was wrong! as you receive arrays from splice, it was necessary to transform it into an singular array, instead of a 2d array.

Instead of that, attach [0] to the end of your bin.push(playlist.splice(playlistIndex[i], 1)), so it would look like:

bin.push(playlist.splice(playlistIndex[i], 1)[0]);

This works because you only splice a single value from the array, but if you were to splice more, you would need to use that concat statement!


Again with the whitespace:

alert(bin.length+" video(s) have been removed: "+bin.join(", ")+".");

into:

alert(bin.length + " video(s) have been removed: " + bin.join(", ") + ".");

  1. Practically speaking, videos to be removed, and by extension, their corresponding indexes in an array should not be predetermined, but rather be selected by the user. In other words, the playlistIndex array should be empty at first, and index numbers should be added to it one by one as the user ticks checkboxes.

Look at this page(It's w3schools, I couldn't find a MDN post on it, sorry) on HTML checkboxes, this may be exactly what you're looking for.

Then, you can iterate over the array of checkboxes and add the indexes of the ones that are checked.

  1. Even after I have ironed out the first problem mentioned above, the current code will remove a selected video immediately after one index number is pushed to the playlistIndex array. So I have to make it so that the code does not start to remove videos until the user has selected every video they want to have removed.

Simply move this to a function! Then attach the function to a submit button on your <form>. It will only get called after the form has been submitted, rather than on addition.

  1. Lastly, before setting the removal process in motion, the code needs to have the user confirm their decision. Ideally, the user should be asked to press "yes" or "no" in a pop-up that goes something like "You are about to remove 7 videos. Are you sure you want to proceed?"

Use confirm(), it lets you ask a user a question and return a boolean response.

For example:

if (!confirm('Are you sure you want to delete these videos?')){
    return;
}
// do deleting here.

Note the reverse condition allows you to get away from an extra layer of conditional over your entire post.

\$\endgroup\$
6
  • \$\begingroup\$ "positive" for loops skips over array elements when changing the array (ex when splicing). a = [0,1,2,3,4]; for (i=0; i<a.length; i++) a.splice(i,1); console.log(a); \$\endgroup\$ – Tibos Dec 14 '15 at 12:56
  • \$\begingroup\$ You're right for j, but not for the first for loop \$\endgroup\$ – Quill Dec 14 '15 at 12:59
  • \$\begingroup\$ @Tibos, those issues are fixed, if you have any more issues with my post feel free to voice them while I'm improving it :-) \$\endgroup\$ – Quill Dec 14 '15 at 13:02
  • \$\begingroup\$ It's fine for now there are no more errors with your advice. \$\endgroup\$ – Tibos Dec 14 '15 at 13:06
  • \$\begingroup\$ Let us continue this discussion in chat. \$\endgroup\$ – Tibos Dec 14 '15 at 13:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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