[solved] Javascript - How to stop repeating myself?


#1

It’s probably a super simple javascript thing for most of you, but I cant seem to get it to work.

Situation

I have a form with four different file-input-fields for images:

  1. sideimage
  2. frontimage
  3. topimage
  4. bottomimage

My insert event for the sideimage looks like this:

Template.product_form_edit.events({
  'change .fileinput--side': function(event, template) {

    FS.Utility.eachFile(event, function(file) {
        var tmpdoc = new FS.File(file);
        tmpdoc.imageType = "sideimage";

        Images.insert(tmpdoc, function (err) {});
    });

    Session.set('sideImageChangeRequest', null);
  }
});

This works like expected.

Problem

Now if I want to create the events for the other images I would copy and paste this whole code-block and just change the “sideimage” and “sideImageChangeRequest”. So basically I repeat most of the code four times.

Example:
My code for the frontimage would look like this:

Template.product_form_edit.events({
  'change .fileinput--front': function(event, template) {

    FS.Utility.eachFile(event, function(file) {
        var tmpdoc = new FS.File(file);
        tmpdoc.imageType = "frontimage";

        Images.insert(tmpdoc, function (err) {});
    });

    Session.set('frontImageChangeRequest', null);
  }
});

So lots of repeating myself here.
How can I make this more efficient?

Thanks for any push in the right direction.


#2

In javascript, you can refactor out a code block into a function and reuse it as you like, passing in arguments.

So you could define an insertImage function that takes two parameters as event and imageType:

insertImage = function(event, imageType) {
    FS.Utility.eachFile(event, function(file) {
        var tmpdoc = new FS.File(file);
        tmpdoc.imageType = imageType;
        Images.insert(tmpdoc, function (err) {});
    });
}

Template.product_form_edit.events({
  'change .fileinput--front': function(event, template) {
    insertImage(event, 'frontimage');  
    Session.set('frontImageChangeRequest', null);
  }
});

But, there is even more you can do to simplify this. Since your inputs are all within the same template, you can create a combined event handler on [type='file'] and distinguish by its class name as follows:

insertImage = function(event, imageType) {
    FS.Utility.eachFile(event, function(file) {
        var tmpdoc = new FS.File(file);
        tmpdoc.imageType = imageType;
        Images.insert(tmpdoc, function (err) {});
    });
}

Template.product_form_edit.events({
  'change [type='file']': function(event, template) {
    classes = event.target.classList;
    if (_.contains(classes, 'fileinput--front')) imageType = 'frontimage'; 
    if (_.contains(classes, 'fileinput--side')) imageType = 'sideimage';  
    insertImage(event, imageType);  
    Session.set('frontImageChangeRequest', null);
  }
});

In this case, what we are doing is universally acting on the change event of any file input, checking the classes that are given to the file input and if any one of the classes matches one of our image type keywords (classes), then we set that imageType. Therefore, instead of creating multiple event handlers, we shorten our code to a single event handler.

To be honest, use of classes for functionality is not a good use for my taste. Classes should be used for presentation through CSS.

You could use custom data attributes which are designed for that purpose. So you could change your file inputs to:

<input type="file" data-image-upload="sideimage">

and then code your event handler like

Template.product_form_edit.events({
  'change data-image-upload': function(event, template) {
    imageType = event.target.getAttribute('data-image-upload');
    insertImage(event, imageType);  
    Session.set('frontImageChangeRequest', null);
  }
});

#3

Hey @serkandurusoy,

thanks a TON for taking the time to help me with this!
I could basically cut the length of my whole code in half. Feels much cleaner to me now.

I’m still amazed by the helpfullness of the Meteor-Community.
This is absolutely great :smile:

I’ll make sure to give somethign back, once I’ve got enough skills to help others out.
Again: Many thanks!


#4

Wow, this means a lot! :smile:

I’ll make sure to give somethign back, once I’ve got enough skills to help others out.

BTW, that’s how I improved my meteor skills; by answering questions here and on SO :smile:


#5

Just wanted to let you know that this type of reply that breaks out “this is the refactoring which answers your questions, but here’s what will expand your skills even more” is amazing. Bookmarked this post even though I don’t need it at this time I know I will later.