3
\$\begingroup\$

I would love a second opinion / another pair of eyes on this.

db.customers.mapReduce(
    function() {
        Array.prototype.getUnique = function() {
            var u = {}, a = [];
            for (var i = 0, l = this.length; i < l; ++i) {
                if (u.hasOwnProperty(this[i])) {
                    continue;
                }
                a.push(this[i]);
                u[this[i]] = 1;
            }
            return a;
        }
        var customer = this;
        this.tags.getUnique().forEach(function(tag) {
            if (customer.status) {
                emit(customer.shop, {
                    "kind": "unique_customers_submitted",
                    "tag": tag.tag,
                });
            } else {
                emit(customer.shop, {
                    "kind": "unique_customers_withheld",
                    "tag": tag.tag,
                });
            }
            emit(customer.shop, {
                "kind": "unique_customers_total",
                "tag": tag.tag,
            });
        })
        this.tags.forEach(function(tag) {
            if (customer.status) {
                emit(customer.shop, {
                    "kind": "total_customers_submitted",
                    "tag": tag.tag,
                });
            } else {
                emit(customer.shop, {
                    "kind": "total_customers_withheld",
                    "tag": tag.tag,
                });
            }
            emit(customer.shop, {
                "kind": "total_customers",
                "tag": tag.tag,
            });
        });
    },
    function(key, sets) {
        var forms = {};
        sets.forEach(function(set) {
            if (typeof forms[set.tag] == "undefined") forms[set.tag] = {};
            if (typeof forms[set.tag][set.kind] == "undefined") forms[set.tag][set.kind] = 0;
            forms[set.tag][set.kind] += 1;
        });
        return forms;
    }, {
        out: "form_numbers",
    },
    function(err, results) {
        console.log(err);
        //console.log(results);
        results.find().toArray(function(err, numbers) {
            console.log(err);
            console.log(JSON.stringify(numbers));
            db.server.close();
        });
    }
);
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Hi, please provide more context to your code, and ideally put a title that says what the code does - everybody posting here is looking for a 2nd pair of eyes ;) \$\endgroup\$ Commented Feb 25, 2014 at 14:39
  • \$\begingroup\$ @Mat'sMug Hey sorry, I'll edit it when I get a chance! Thanks for the suggestion!! \$\endgroup\$ Commented Feb 25, 2014 at 16:53

1 Answer 1

6
\$\begingroup\$

I'm not familiar with MR best practices, but here are some JS-specific comments.

Key Points

  • DRY - that emit code is repeated all over the place and has only slight changes - parameterize what varies and put it all in a function

  • You are repeating your loops - either do everything in one loop for better performance (and keep in mind that for is faster than forEach, or go fully functional (see next)

  • You are essentially doing a filter within your loop - use filter to remove the if logic from within the loop (or see previous). If you want to go even more functional, maybe you should use underscorejs or lo-dash, which both have a unique function already.

  • Never change the prototype of a builtin unless you have already checked if the method exists (like a polyfill) and/or you have a really, really good reason to do so. Mostly, just don't.

  • Declare functions so that they are not re-created (unless necessary)

  • You should probably close your db connection ASAP

  • Oh, and name your inline functions to provide better documentation

A simple refactoring hosted as a gist for easy forking

function getUnique() {
    var u = {}, a = [];
    for (var i = 0, l = this.length; i < l; ++i) {
        if (u.hasOwnProperty(this[i])) {
            continue;
        }
        a.push(this[i]);
        u[this[i]] = 1;
    }
    return a;
};
function emitHelper(kind, shop, tag) {
    emit(shop, {
        "kind": kind,
        "tag": tag,
    });
};
db.customers.mapReduce(
    function mapCustomerToShopWithKindAndTag() {
        var customer = this;
        var kind;
        getUnique(this.tags).forEach(function(tag) {
            kind = customer.status ? "unique_customers_submitted" : "unique_customers_withheld";
            emitHelper(kind, customer.shop, tag.tag);
            emitHelper("unique_customers_total", customer.shop, tag.tag);
        });
        this.tags.forEach(function(tag) {
            kind = customer.status ? "total_customers_submitted" : "total_customers_withheld";
            emitHelper(kind, customer.shop, tag.tag);
            emitHelper("total_customers_total", customer.shop, tag.tag);
        });
    },
    function reduceToCountOfKindsPerTag(key, sets) {
        var forms = {};
        sets.forEach(function(set) {
            if (typeof forms[set.tag] == "undefined") forms[set.tag] = {};
            if (typeof forms[set.tag][set.kind] == "undefined") forms[set.tag][set.kind] = 0;
            forms[set.tag][set.kind] += 1;
        });
        return forms;
    }, {
        out: "form_numbers",
    },
    function(err, results) {
        console.log(err);
        //console.log(results);
        results.find().toArray(function(err, numbers) {
            db.server.close(); // Maybe wrap in a try/catch too
            console.log(err);
            console.log(JSON.stringify(numbers));
        });
    }
);
\$\endgroup\$
1
  • 1
    \$\begingroup\$ This is a very nice answer, keep it up! \$\endgroup\$
    – syb0rg
    Commented Mar 1, 2014 at 23:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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