current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I'm looking to change a single jQuery plugin option when the window hits certain breakpoints.

The code below works, however I'm conscious of repeating code and feel there is a more elegant way writing code for examples like this.

My initial thoughts would be to store the first call in a variable.

$(window).smartresize(function () {

        if ( config.wWidth >= 768 && config.wWidth <= 1024 ) {

            $('#va-accordion').vaccordion({
                accordionW       : config.wWidth,
                accordionH       : config.wHeight,
                visibleSlices    : 2,
                expandedHeight   : 600,
                expandedWidth    : 1000,
                contentAnimSpeed : 200
            });

        } else if ( config.wWidth < 768 ) {

            $('#va-accordion').vaccordion({
                accordionW       : config.wWidth,
                accordionH       : config.wHeight,
                visibleSlices    : 1,
                expandedHeight   : 600,
                expandedWidth    : 1000,
                contentAnimSpeed : 200
            });

        } else {

            $('#va-accordion').vaccordion({
                accordionW       : config.wWidth,
                accordionH       : config.wHeight,
                visibleSlices    : 3,
                expandedHeight   : 600,
                expandedWidth    : 1000,
                contentAnimSpeed : 200
            });
        }

    });

    $(window).trigger("smartresize");
share|improve this question

2 Answers 2

up vote 4 down vote accepted 
$(window).smartresize(function () {

  var chosenSettings;

  // We could map out the defaults first.
  var defaults = {
    accordionW: config.wWidth,
    accordionH: config.wHeight,
    visibleSlices: 0,
    expandedHeight: 600,
    expandedWidth: 1000,
    contentAnimSpeed: 200
  };

  // Then, depending on the conditions, create an object of the modifications
  // This way, it's easier to add in more differences.
  if (config.wWidth >= 768 && config.wWidth <= 1024) {
    chosenSettings = {visibleSlices: 2};
  } else if (config.wWidth < 768) {
    chosenSettings = {visibleSlices: 1};
  } else {
    chosenSettings = {visibleSlices: 3};
  }

  // Then use extend to create an object with the chosen settings
  var settings = $.extend({}, defaults, chosenSettings);
  $('#va-accordion').vaccordion(settings);  

// Assuming smartresize returns the same object, we can chain trigger.
}).trigger('smartresize');
share|improve this answer
    
This is exactly what I was after... it will be a great help for my future work as well, thank you –  Sam Holguin May 7 '14 at 8:18
    
That's a great solution Joseph! Let me suggest moving the if block to a method that receives config and returns the chosenSettings. I think that will be even better! :) –  Juliano May 7 '14 at 14:33
    
@Juliano I didn't do that because I assumed this function gets called on resize, and a function inside this would be bad for performance as it is created everytime it is called. –  Joseph the Dreamer May 7 '14 at 16:02
    
I didn't think about it, excellent point! –  Juliano May 7 '14 at 17:17
up vote 3 down vote

You might extract your code to a function to remove the repeated code, passing the values ​​that change as parameters:

$(window).smartresize(function () {

    if (config.wWidth >= 768 && config.wWidth <= 1024) {
        createVaccordion(config, 2);

    } else if (config.wWidth < 768) {
        createVaccordion(config, 1);

    } else {
        createVaccordion(config, 3);
    }

    function createVaccordion(config, slices) {
        $('#va-accordion').vaccordion({
            accordionW: config.wWidth,
            accordionH: config.wHeight,
            visibleSlices: slices,
            expandedHeight: 600,
            expandedWidth: 1000,
            contentAnimSpeed: 200
        });
    }
});

$(window).trigger("smartresize");
share|improve this answer
    
This is a method I hadn't considered, thank you. I've gone with an alternative solution, but marked this up. –  Sam Holguin May 7 '14 at 8:19

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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