HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Selecting multiple images

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
multipleimagesselecting

Problem

I wrote this code to allow users to select multiple images, and each image can be selected multiple times. It feels convoluted, though, and prone to errors, such as forgetting to do something associated with an action.

How can I make it more object-oriented or reusable?

```
var images = [];

function removeImage(image) {
while((index = images.indexOf(image)) !== -1) {
images.splice(index, 1);
}
}
function decrementImage(image) {
var index = images.indexOf(image);
images.splice(index, 1);
}

function countOccurrences(image) {
var occurrences = 0;
for (i = 0; i < images.length; i++) {
if (images[i] === image) occurrences += 1
}
return occurrences;
}

$('#image-container').on('click', 'img', function () {
var image = $(this),
source = image.attr('src');
image.data('selected', true).addClass('selected').siblings('.controls').show();
images.push(source);
image.siblings('span.count').html(countOccurrences(source));
});

$('#image-container').on('mouseleave', '.image-wrapper', function () {
$(this).children('.controls').hide();
});

$('#image-container').on('mouseenter', 'img', function () {
var image = $(this),
controls = image.siblings('.controls');
if (image.data('selected') === true) {
controls.show();
}
});

$('#image-container').on('click', '[data-action="remove"]', function () {
var controls = $(this).parents('div.controls'),
image = controls.siblings('img')
source = image.attr('src');
image.data('selected', false).removeClass('selected');
controls.hide();
removeImage(source);
image.siblings('span.count').html(countOccurrences(source));
});

$('#image-container').on('click', '[data-action="increment"]', function () {
var image = $(this).parents('.controls').siblings('img'),
source = image.attr('src');
images.push(source);
image.siblings('span.count').html(countOccurrences(source

Solution

Your code is actually very good.

I'd advice against using OOP there. It's an overused tool which, when not necessary, only makes things more complex. JavaScript works very well without it. I'd recommend a more functional style. So, if you want to make your code more reusable, you could abstract your functions to work with any kind of collection. You could also use existing functions to define others:

function removeFirst(array,obj) {
    array.splice(array.indexOf(obj), 1);
};
function without(array,obj){
    return array.filter(function(element){
        return element !== obj;
    });
};
function count(array,obj){
    return array.length - without(array,obj).length;
};


Then, counting an image would be, for example:

count(images,my_image);


But before I'd suggest checking some functional JavaScript libraries such as underscore.js, which already implement similar functions in very clever manners. Using them in your daily code will make it smaller, simpler, more readable and robust.

Code Snippets

function removeFirst(array,obj) {
    array.splice(array.indexOf(obj), 1);
};
function without(array,obj){
    return array.filter(function(element){
        return element !== obj;
    });
};
function count(array,obj){
    return array.length - without(array,obj).length;
};
count(images,my_image);

Context

StackExchange Code Review Q#23489, answer score: 2

Revisions (0)

No revisions yet.