CODEWITHSUNDEEP
javascriptjquery
MEM
MEM

Reputation: 31337

Javascript - how can I properly shrink this code

Regarding the browser version, the player height should change

if older then ie9 
  //height is fixed
else 
  //height is auto

The above code is working but, it's the worst thing I've seen, because I do repeat myself again and again, when only one line changes on this conditional.

<script type="text/javascript"> 
    $(document).ready(function() {

        if ($.browser.msie && $.browser.version.substr(0,1)<9) {
            $("#jquery_jplayer_1").jPlayer({
            ready: function () 
            {
                $(this).jPlayer("setMedia", 
                {
                    m4v: "/video/videoK.mp4",
                    ogv: "/video/videoK.ogg"
                }).jPlayer("play");

                $('article.about-k').hide();
                olark('api.box.hide');
            },
            swfPath: "/scripts/",
            supplied: "m4v, ogv",
            size: {
                width: "100%",
                height: "400px" // THE ONLY CHANGE IS HERE
            },
            backgroundColor: "#fff",
            click: function() {
                $(this).jPlayer("play");
            },
            ended: function() {
                $('.jplayer-k').hide();
                $('article.about-k').show();
            }
        })
      } else {
        $("#jquery_jplayer_1").jPlayer({
            ready: function () 
            {
                $(this).jPlayer("setMedia", 
                {
                    m4v: "/video/videoK.mp4",
                    ogv: "/video/videoK.ogg"
                }).jPlayer("play");

                $('article.about-k').hide();
                olark('api.box.hide');
            },
            swfPath: "/scripts/",
            supplied: "m4v, ogv",
            size: {
                width: "100%",
                height: "auto" // THE ONLY CHANGE IS HERE
            },
            backgroundColor: "#fff",
            click: function() {
                $(this).jPlayer("play");
            },
            ended: function() {
                $('.jplayer-k').hide();
                $('article.about-k').show();
            }
        })
      }

    });
</script>

As noted by the comments, the only change is on ONE single line. How can I remake this, without stupidly repeating myself ?

 height: "auto" //THIS IS THE ONLY DIFFERENCE!

Please advice

Upvotes: 1

Views: 153

Answers (4)

LetterEh
LetterEh

Reputation: 26696

Try:

var playerObj = { ready : function () { /* ... */ }, size : /* AUTO */ };
if (/*stupid IE TEST*/) {
    playerObj.size.height = "...";
}

$("#jquery_jplayer_1").jPlayer(playerObj);

Separate your configuration from your processes, and it becomes much less of a hassle.

Also, like @Bergi said, a ternary-operation:

value = (test_with_optional_parentheses) ? passed_value : failed_value;

...or in this case, as an object parameter:

{ value : (test_with_optional_parentheses) ? passed_value : failed_value };

Works perfectly well for changing one thing, inline, in the middle of a configuration object.

Keep my advice in mind for organizing large blocks of code, when you start confusing what you need to do with what you're currently doing. Keep Bergi's advice in mind for when you need to set one single value (at a time), based on a simple test.

Upvotes: 0

Nelson Ben&#237;tez Le&#243;n
Nelson Ben&#237;tez Le&#243;n

Reputation: 50563

It could be like this:

<script type="text/javascript"> 
    $(document).ready(function() {

        if('<?= $cookie; ?>' > '1' || $(window).width() <= 768) {
          $('.jplayer-k').remove();
          $('article.about-k').show();
       }
       height = $.browser.msie && parseInt($.browser.version, 10) < 9 ? '400px' : 'auto';
         $("#jquery_jplayer_1").jPlayer({
            ready: function () 
            {
                $(this).jPlayer("setMedia", 
                {
                    m4v: "/video/videoK.mp4",
                    ogv: "/video/videoK.ogg"
                }).jPlayer("play");

                $('article.about-k').hide();
                olark('api.box.hide');
            },
            swfPath: "/scripts/",
            supplied: "m4v, ogv",
            size: {
                width: "100%",
                height: height
            },
            backgroundColor: "#fff",
            click: function() {
                $(this).jPlayer("play");
            },
            ended: function() {
                $('.jplayer-k').hide();
                $('article.about-k').show();
            }
        })
    });
</script>

UPDATE:
I added to code above the Fabriccio Matte suggestion so your code dont fail with IE10

Upvotes: 0

Bergi
Bergi

Reputation: 664503

That's quite simple with a ternary operator:

$("#jquery_jplayer_1").jPlayer({
    ready: function () {
        $(this).jPlayer("setMedia", {
            m4v: "/video/videoK.mp4",
            ogv: "/video/videoK.ogg"
        }).jPlayer("play");

        $('article.about-k').hide();
        olark('api.box.hide');
    },
    swfPath: "/scripts/",
    supplied: "m4v, ogv",
    size: {
        width: "100%",
        height: ($.browser.msie && $.browser.version.substr(0, 1) < 9)
          ? "400px"
          : "auto"
    },
    backgroundColor: "#fff",
    click: function () {
        $(this).jPlayer("play");
    },
    ended: function () {
        $('.jplayer-k').hide();
        $('article.about-k').show();
    }
})

If you don't like that (e.g. because of a more complex condition), you still can use a simple variable:

var height = "auto";
if (/* IE too old */)
    height = "400px";
$…({
      … // huge config object
      height: height,
      …
});

Upvotes: 4

furtive
furtive

Reputation: 1689

Why not:

   <script type="text/javascript"> 
        $(document).ready(function() {

            if('<?= $cookie; ?>' > '1' || $(window).width() <= 768) {
              $('.jplayer-k').remove();
              $('article.about-k').show();
           }

            $("#jquery_jplayer_1").jPlayer({
            ready: function () 
            {
                $(this).jPlayer("setMedia", 
                {
                    m4v: "/video/videoK.mp4",
                    ogv: "/video/videoK.ogg"
                }).jPlayer("play");

                $('article.about-k').hide();
                olark('api.box.hide');
            },
            swfPath: "/scripts/",
            supplied: "m4v, ogv",
            size: {
                width: "100%",
                height: ($.browser.msie && $.browser.version.substr(0,1)<9) ? "400px" : "auto"
            },
            backgroundColor: "#fff",
            click: function() {
                $(this).jPlayer("play");
            },
            ended: function() {
                $('.jplayer-k').hide();
                $('article.about-k').show();
            }
        })

Upvotes: 0

Related Questions