Return to Answer

It's easier if you set event handlers within javascript rather than inside the html code; this way it is simpler to pass variables to a single event handler. I don't understand why each of your event handlers has var data=ev.dataTransfer.getData but then doesn't do anything with the data; maybe this is something you will use later? And each of the top left and top right images is calling two separate functions when dropped (dragRightdropRight1(event),dragLeftdropRight1(event) etc), but as far as I can see the two functions do the same thing.

Anyway you can simplify as follows, with a single function drop that does something different depending on the name of the box (topLeft, topRight) and the image number (0-4).

var boxes = ['topLeft', 'topRight', 'bottomLeft', 'bottomRight'],
    images = {};

// Make a dictionary-like object containing all the image elements
for (var i = 0; i < boxes.length; i++) {
    images[boxes[i]] = document.getElementById('box' + (i + 1)).getElementsByTagName('img');
}

// Set event handlers for the 1st two boxes, topLeft and topRight
for (var i = 0; i < 5; i++) {
    images['topLeft'][i].ondrop = getDropFunction(ev, 'topLeft', i);
    images['topRight'][i].ondrop = getDropFunction(ev, 'topRight', i);
}

// Needed for reasons of variable scope
function getDropFunction(ev, boxName, imageNumber) {
    return function() {
        drop(ev, boxName, imageNumber);
    };
}

// Deal with the drop event, depending on which box and image was dropped
function drop(ev, boxName, imageNumber) {
    var nextImageNumber = (imageNumber + 1) % 5;
    ev.preventDefault();
    images[boxName][imageNumber].style.display = 'none';
    images[boxName][nextImageNumber].style.display = 'block';
    // as far as I can see this is all your code is doing
    // but you may want to add something that gets the data
    // from the ev and deals with it
}

It's easier if you set event handlers within javascript rather than inside the html code; this way it is simpler to pass variables to a single event handler. I don't understand why each of your event handlers has var data=ev.dataTransfer.getData but then doesn't do anything with the data; maybe this is something you will use later? And each of the top left and top right images is calling two separate functions when dropped (dragRightdropRight1(event),dragLeftdropRight1(event) etc), but as far as I can see the two functions do the same thing.

Anyway you can simplify as follows, with a single function drop that does something different depending on the name of the box (topLeft, topRight) and the image number (0-4).

var boxes = ['topLeft', 'topRight', 'bottomLeft', 'bottomRight'],
    images = {};

// Make a dictionary-like object containing all the image elements
for (var i = 0; i < boxes.length; i++) {
    images[boxes[i]] = document.getElementById('box' + (i + 1)).getElementsByTagName('img');
}

// Set event handlers for the 1st two boxes, topLeft and topRight
for (var i = 0; i < 5; i++) {
    images['topLeft'][i].ondrop = getDropFunction(ev, 'topLeft', i);
    images['topRight'][i].ondrop = getDropFunction(ev, 'topRight', i);
}

// Needed for reasons of variable scope
function getDropFunction(ev, boxName, imageNumber) {
    return function() {
        drop(ev, boxName, imageNumber);
    };
}

// Deal with the drop event, depending on which box and image was dropped
function drop(ev, boxName, imageNumber) {
    var nextImageNumber = (imageNumber + 1) % 5;
    ev.preventDefault();
    images[boxName][imageNumber].style.display = 'none';
    images[boxName][nextImageNumber].style.display = 'block';
    // as far as I can see this is all your code is doing
    // but you may want to add something that gets the data
    // from the ev and deals with it
}

If you use this kind of solution you can remove the ondrop attributes from the images in the HTML.

It's easier if you set event handlers within javascript rather than inside the html code; this way it is simpler to pass variables to a single event handler. I don't understand why each of your event handlers has var data=ev.dataTransfer.getData but then doesn't do anything with the data; maybe this is something you will use later? And each of the top left and top right images is calling two separate functions when dropped (dragRightdropRight1(event),dragLeftdropRight1(event) etc), but as far as I can see the two functions do the same thing.

Anyway you can simplify as follows, with a single function drop that does something different depending on the name of the box (topLeft, topRight) and the image number (0-4).

var boxes = ['topLeft', 'topRight', 'bottomLeft', 'bottomRight'],
    images = {};

// Make a dictionary-like object containing all the image elements
for (var i = 0; i < boxes.length; i++) {
    images[boxes[i]] = document.getElementById('box' + (i + 1)).getElementsByTagName('img');
}

// Set event handlers for the 1st two boxes, topLeft and topRight
for (var i = 0; i < 5; i++) {
    images['topLeft'][i].ondrop = function(ev) {
        drop(ev, 'topLeft', i);
    }
    images['topRight'][i].ondrop = function(ev) {
        drop(ev, 'topRight', i);
    }
}

// The event handler itself
function drop(ev, boxName, imageNumber) {
    var nextImageNumber = (imageNumber + 1) % 5;
    ev.preventDefault();
    images[boxName][imageNumber].style.display = 'none';
    images[boxName][nextImageNumber].style.display = 'block';
    // as far as I can see this is all your code is doing
    // but you may want to add something that gets the data
    // from the ev and deals with it
}

It's easier if you set event handlers within javascript rather than inside the html code; this way it is simpler to pass variables to a single event handler. I don't understand why each of your event handlers has var data=ev.dataTransfer.getData but then doesn't do anything with the data; maybe this is something you will use later? And each of the top left and top right images is calling two separate functions when dropped (dragRightdropRight1(event),dragLeftdropRight1(event) etc), but as far as I can see the two functions do the same thing.

Anyway you can simplify as follows, with a single function drop that does something different depending on the name of the box (topLeft, topRight) and the image number (0-4).

var boxes = ['topLeft', 'topRight', 'bottomLeft', 'bottomRight'],
    images = {};

// Make a dictionary-like object containing all the image elements
for (var i = 0; i < boxes.length; i++) {
    images[boxes[i]] = document.getElementById('box' + (i + 1)).getElementsByTagName('img');
}

// Set event handlers for the 1st two boxes, topLeft and topRight
for (var i = 0; i < 5; i++) {
    images['topLeft'][i].ondrop = getDropFunction(ev, 'topLeft', i);
    images['topRight'][i].ondrop = getDropFunction(ev, 'topRight', i);
}

// Needed for reasons of variable scope
function getDropFunction(ev, boxName, imageNumber) {
    return function() {
        drop(ev, boxName, imageNumber);
    };
}

// Deal with the drop event, depending on which box and image was dropped
function drop(ev, boxName, imageNumber) {
    var nextImageNumber = (imageNumber + 1) % 5;
    ev.preventDefault();
    images[boxName][imageNumber].style.display = 'none';
    images[boxName][nextImageNumber].style.display = 'block';
    // as far as I can see this is all your code is doing
    // but you may want to add something that gets the data
    // from the ev and deals with it
}

It's easier if you set event handlers within javascript rather than inside the html code; this way it is simpler to pass variables to a single event handler. I don't understand why each of your event handlers has var data=ev.dataTransfer.getData but then doesn't do anything with the data; maybe this is something you will use later? And each of the top left and top right images is calling two separate functions when dropped (dragRightdropRight1(event),dragLeftdropRight1(event) etc), but as far as I can see the two functions do the same thing.

Anyway you can simplify as follows, with a single function drop that does something different depending on the name of the box (topLeft, topRight) and the image number (0-4).

var boxes = ['topLeft', 'topRight', 'bottomLeft', 'bottomRight'],
    images = {};

// Make a dictionary-like object containing all the images
for (var i = 0; i < boxes.length; i++) {
    images[boxes[i]] = document.getElementById('box' + (i + 1)).getElementsByTagName('img');
}

// Set event handlers for the 1st two boxes, topLeft and topRight
for (var i = 0; i < 2; i++) {
    for (var j = 0; j < images[i].length; j++) {
        images[boxes[i]][j].ondrop = function(ev) {
            drop(ev, boxes[i], j);
        }
    }
}

// The event handler itself
function drop(ev, boxName, imageNumber) {
    var nextImageNumber = (imageNumber + 1) % 5;
    ev.preventDefault();
    images[boxName][imageNumber].style.display = 'none';
    images[boxName][nextImageNumber].style.display = 'block';
    // as far as I can see this is all your code is doing
    // but you may want to add something that gets the data
    // from the ev and deals with it
}

It's easier if you set event handlers within javascript rather than inside the html code; this way it is simpler to pass variables to a single event handler. I don't understand why each of your event handlers has var data=ev.dataTransfer.getData but then doesn't do anything with the data; maybe this is something you will use later? And each of the top left and top right images is calling two separate functions when dropped (dragRightdropRight1(event),dragLeftdropRight1(event) etc), but as far as I can see the two functions do the same thing.

Anyway you can simplify as follows, with a single function drop that does something different depending on the name of the box (topLeft, topRight) and the image number (0-4).

var boxes = ['topLeft', 'topRight', 'bottomLeft', 'bottomRight'],
    images = {};

// Make a dictionary-like object containing all the image elements
for (var i = 0; i < boxes.length; i++) {
    images[boxes[i]] = document.getElementById('box' + (i + 1)).getElementsByTagName('img');
}

// Set event handlers for the 1st two boxes, topLeft and topRight
for (var i = 0; i < 5; i++) {
    images['topLeft'][i].ondrop = function(ev) {
        drop(ev, 'topLeft', i);
    }
    images['topRight'][i].ondrop = function(ev) {
        drop(ev, 'topRight', i);
    }
}

// The event handler itself
function drop(ev, boxName, imageNumber) {
    var nextImageNumber = (imageNumber + 1) % 5;
    ev.preventDefault();
    images[boxName][imageNumber].style.display = 'none';
    images[boxName][nextImageNumber].style.display = 'block';
    // as far as I can see this is all your code is doing
    // but you may want to add something that gets the data
    // from the ev and deals with it
}