1
\$\begingroup\$

I implemented search module, but I'm a novice in programming and I'm not sure about public and private functions that I coded. Did I do it right?

var data = require('./data');


function My_module(string) {
    var data_size_ = data.length;

    /**
     * @param {Array.<Object>} data.
     * @return {Array.<Object>} result.
     */

    var reformat_ = function() {
        var result = new Array(data_size_);

        var first_name;
        var last_name;
        var domain;

        for (var i = 0; i < data_size_; ++i) {
            first_name = data[i].first_name.toLowerCase();
            last_name = data[i].last_name.toLowerCase();
            domain = data[i].domain.toLowerCase();

            result[i] = {
                'first_name': first_name,
                'last_name': last_name,
                'domain': domain,
            };
        }

        return result;
    };

    var formatted_data_ = reformat_();



    /**
     * A search function.
     * @return {Array.<Object>} Result
     */

    this.search = function(string) {
        var result = [];
        var len = string.length;

        for (var i = 0; i < data_size_; ++i) {
            if (formatted_data_[i].first_name.slice(0, len) === string ||
                formatted_data_[i].last_name.slice(0, len) === string ||
                formatted_data_[i].domain.slice(0, len) === string) {
                result.push(formatted_data_[i]);
            }
        }

        if (result) {
            return result;
        }
    }

}


module.exports = My_module;
\$\endgroup\$
1
\$\begingroup\$

Rather than creating a function named My_module, and assigning it to module.exports, you can just directly assign a function to module.exports, like this:

module.exports = function() {
    ...
}

Secondly, why are you ending all your functions and variables in My_module with an underscore, _? It doesn't do anything, and if it's not serving a real naming purpose, it can be removed.

\$\endgroup\$
  • \$\begingroup\$ The last part of you statement is wrong. It will only return result if it is truthy not just true and the result will always be truthy because it is assigned to []. Also doing return null at the end is not preferred in js because by default it will return undefined unless you need to return null undefined would be acceptable. \$\endgroup\$ – pllee Jul 9 '15 at 17:42
  • \$\begingroup\$ @pllee Removed that. \$\endgroup\$ – Ethan Bierlein Jul 9 '15 at 17:43
  • 1
    \$\begingroup\$ got it, I wish I started coding around your age :) \$\endgroup\$ – pllee Jul 9 '15 at 17:51
  • \$\begingroup\$ @pllee Thanks! An upvote would be appreciated if you thought the answer was good. :) \$\endgroup\$ – Ethan Bierlein Jul 9 '15 at 17:52
  • \$\begingroup\$ It is not bad but it is not really commenting on the more major concerns imo. If I have some time I'll answer with my concerns. \$\endgroup\$ – pllee Jul 9 '15 at 17:58

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.