2
\$\begingroup\$

I created a plug-in powered by jQuery that simplifies the developer's life to create responsive applications. It's flexible and I want to give to users a good syntax to write.

How can I improve this?

GitHub

; (function ($) {
    var debug = {
        ranges: [],
        refreshRanges: function(ranges) {
            this.ranges = ranges;
        },
        invoke: function(reference) {
            console.log(this.warnings[reference].apply());
        },
        clean: function() {
            this.ranges = undefined;
        },
        warnings: {
            inRange: function () {
                return 'You\'re in range between ' + debug.ranges[0] + 'px and ' + debug.ranges[1] + 'px.';
            },
            outOfRange: function() {
                return 'You\'re out of range between ' + debug.ranges[0] + 'px and ' + debug.ranges[1] + 'px.';
            }
        }
    };

    var screen = {
            inRange: undefined,
            outOfRange: undefined,
            firstTimeResponsive: true,
            sensibilize: function(ranges, action, callback, debugging) {
                if (this.isInRange(ranges) === true) {
                    this.outOfRange = false;

                    if (this.inRange === false || (typeof this.inRange === 'undefined' && this.firstTimeResponsive === true)) {
                        action(device);

                        this.inRange = true;
                        this.firstTimeResponsive = false;

                        if (debugging === true) {
                            debug.refreshRanges(ranges);
                            debug.invoke('inRange');
                            debug.clean();
                        }
                    };
                } else {
                    this.inRange = false;

                    if (this.outOfRange === false || (typeof this.outOfRange === 'undefined' && this.firstTimeResponsive === true)) {
                        if (this.firstTimeResponsive === false) {
                            callback(device);
                        }

                        this.outOfRange = true;
                        this.firstTimeResponsive = false;

                        if (debugging === true) {
                            debug.refreshRanges(ranges);
                            debug.invoke('outOfRange');
                            debug.clean();
                        }
                    };
                }
            },
            isInRange: function(ranges) {
                if (this.reached(ranges)) {
                    return true;
                };
            },
            reached: function(range) {
                var windowWidth = $(window).width();

                if (windowWidth >= range[0] && windowWidth <= range[1]) {
                    return true;
                }
            }
        };

    var device = {
        identification: navigator.userAgent,
        isMobile: function() {
            return /mobi/i.test(navigator.userAgent);
        },
        Android: function() {
            return navigator.userAgent.match(/Android/i);
        },
        BlackBerry: function() {
            return navigator.userAgent.match(/BlackBerry/i);
        },
        iOS: function() {
            return navigator.userAgent.match(/iPhone|iPad|iPod/i);
        },
        Opera: function() {
            return navigator.userAgent.match(/Opera Mini/i);
        },
        Windows: function() {
            return navigator.userAgent.match(/IEMobile/i);
        },
        any: function() {
            return (device.Android() || device.BlackBerry() || 
                    device.iOS() || device.Opera() || device.Windows());
        }
    };

    $.sensitive = function(ranges, action, callback, options) {
        var defaults = {
            debugging: false,
            handheldDevicesOnly: false,
            ultimateScreenWidth: 15360
        };

        var plugin = this;
        plugin.settings = $.extend({}, defaults, options);

        if (typeof ranges[1] === 'undefined') {
            ranges[1] = plugin.settings.ultimateScreenWidth;
        };

        if (plugin.settings.handheldDevicesOnly === true && device.isMobile() === false) {
            return null;
        };

        $(window).on('resize', function() {
            screen.sensibilize(ranges, action, callback, plugin.settings.debugging);
        }).trigger('resize');
    };
}(jQuery));
\$\endgroup\$
7
\$\begingroup\$

My 2 cents :

  • Why jQuery, libraries targetting mobile should not require jQuery, #perfmatters
  • Your github project should have a production version where debug is excluded, your code is 4215 bytes with debug feature, 3014 without debugging ( removing all debugging related code )
  • Your code maintains both the value of inRange and outOfRange, you should only need 1 ?
  • this.firstTimeResponsive === true can be replaced with this.firstTimeResponsive
  • this.inRange === false || (typeof this.inRange === 'undefined') can be !this.inRange
  • isInRange seems pointless, it's a wrapper around reached without added value ?
  • ranges is an unfortunate parameter name, since you only pass 1 range ( from/to pair )
  • action and callback are misleading as parameter names, how about inRangeCallback and outOfRangeCallback ?

From a design perspective, why do you track firstTimeResponse and also track resizing of the browser, it seems pointless. I think you need to test resizing.

Whereas this review seems harsh, I think the library can be put to good use, but it needs some serious clean up.

\$\endgroup\$
  • \$\begingroup\$ Do you think that debugging parameter is necessary? I mean, isn't an argument, it's just an option. sensibilize: function(range, inRangeCallback, outOfRangeCallback, debugging) { ... } \$\endgroup\$ – Guilherme Oderdenge Dec 18 '13 at 10:29
  • \$\begingroup\$ It's 25% less code without debugging code. \$\endgroup\$ – konijn Dec 18 '13 at 12:00
  • \$\begingroup\$ I followed your hint — I removed debugging. If you want to see the new version, jump in. \$\endgroup\$ – Guilherme Oderdenge Dec 19 '13 at 11:52
3
\$\begingroup\$

I know names aren't hugely important but my 2 cents in addition to having already written some of what @tomdemuyt said (which I editted out):

  • clean isn't very descriptive. It's also inconsistent. Based upon other names, a better fit would be clearRanges

  • You don't always have to do === false or === true Please use truthy/falsiness. You're already doing this by not returning a value from your functions that either return true, or undefined. Undefined being falsy.

  • As tomdemuyt said, you can simplify some of your functions. Instead of

    if (windowWidth >= range[0] && windowWidth <= range[1]) {
                return true;
            }
    

    You could use:

    return windowWidth >= range[0] && windowWidth <= range[1];
    

I agree with tomdemuyt that requiring this to be used with jQuery seems to contradict your description of it being "lightweight". Most of the code doesn't reference jQuery, I'd look into factoring out the jQuery dependence.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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