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 2 down vote favorite

For a plugin I'm writing in jQuery I have two optional parameters. For each parameter I do a check for its value. However, I'm curious if I can't write it shorter. Here is the fiddle and here's the code.

(function ($) {
    $.rgbGenerator = function (color, options) {
        var args = $.extend({
            returnAsObj: false,
            addAlpha: false
        }, options);

        var d = $("<div>");
        d.css("color", color).appendTo("body");
        var rgb = d.css("color");
        rgb = rgb.replace(/(^rgba?\(|\)$|\s)/g, "").split(",").map(Number);
        d.remove();
        if (args.addAlpha === false) {
            if (rgb.length == 3) {
                if (!args.returnAsObj === true) {
                    return "rgb(" + rgb[0] + "," + rgb[1] + "," + rgb[2] + ")";
                } else {
                    return {
                        r: rgb[0],
                        g: rgb[1],
                        b: rgb[2]
                    };
                }
            } else if (rgb.length == 4) {
                if (!args.returnAsObj === true) {
                    return "rgba(" + rgb[0] + "," + rgb[1] + "," + rgb[2] + "," + rgb[3] + ")";
                } else {
                    return {
                        r: rgb[0],
                        g: rgb[1],
                        b: rgb[2],
                        a: rgb[3]
                    };
                }
            }
        } else {
            if (!args.returnAsObj === true) {
                return "rgba(" + rgb[0] + "," + rgb[1] + "," + rgb[2] + "," + args.addAlpha + ")";
            } else {
                return {
                    r: rgb[0],
                    g: rgb[1],
                    b: rgb[2],
                    a: args.addAlpha
                };
            }
        }
    };
})(jQuery);

Basically, what it does is

  1. Check args.addAlpha whether it's false (default)
  2. Check the length of rgb (when it's 3 it's RGB, when it's 4 it's rgba)
  3. Check args.returnAsObj. If it's set to true, return an object rather than a string

Especially the part where I check the length of rgb seems unnecessary, but I'm not sure how I could write it any other way. Something like this would be nice:

return {
    r: rgb[0],
    g: rgb[1],
    b: rgb[2],
    a: function () {
        if (rgb.length == 4 || !args.addAlpha === false) {
            return rgb[3] || args.addAlpha;
        }
    }
};

But I assume that's not possible.

share|improve this question

1 Answer 1

up vote 1 down vote accepted

You are right that returning an object where a is a function would not work, but it would work to return the result of a call to a function!

You can also restructure things a bit to reduce duplicated code. By having a starting string or a starting object, and optionally adding to that string or object if some conditions are fulfilled.

This is untested code, but I think it should do the same as your original.

function result(addAlpha, rgb, asObject) {
    if (asObject) {
        var obj = { r: rgb[0], g: rgb[1], b: rgb[2] };
        if (addAlpha !== false) {
            obj.a = addAlpha;
        }
        else if (rgb.length == 4) {
            obj.a = rgb[3];
        }
        return obj;
    }

    var a = (addAlpha !== false) || (rgb.length == 4) ? "a" : "";
    var alpha = "";
    if (addAlpha !== false) alpha = "," + addAlpha;
    else if (rgb.length == 4) alpha = "," + rgb[3];
    return "rgb" + a + "(" + rgb[0] + "," + rgb[1] + "," + rgb[2] + alpha + ")";
}
share|improve this answer
    
Is it possible to keep this in the plugin format? –  Bram Vanroy May 31 at 17:09
1  
@BramVanroy I am sure it is, somehow, but that goes outside the scope of my answer. Otherwise you can just copy the contents of the result method I have written here to your $.rgbGenerator = function (color, options) { –  Simon Forsberg♦ May 31 at 17:11
    
Thank you, after some fiddling I got it to work. Thanks! jsfiddle.net/BramVanroy/qog37z7g –  Bram Vanroy May 31 at 18:45

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.