7
\$\begingroup\$

I'm considering releasing a library as a bower module. Are there issues with code quality, missing test cases, that need to be addressed first? Perhaps it's not a good candidate for public release. If so, why not?

What level of documentation would you want for something like this? I've seen some open source modules publish annotated source, in addition to API documentation. Is annotated source documentation overkill for something like this?

Library Details:

  • The library provides a simple object persistence layer for Angularjs apps using a local Pouchdb database. It is $digest aware and promise based. The public API consists of
    • setType(type, fn) //register prototype
    • newObj() //new object from registered prototype
    • saveObj(obj) // insert and update
    • getAll(type)
    • getById(type,id)
    • getByName(type,name)
    • getByGQL(query) //TODO: support arbitrary find conditions via Google Query Language
    • deleteObj(obj)
    • deleteById(id)
  • Because the library supports objects that may include functions, the library requires a prototype for each type be registered with the library. The prototype is used to re-hydrate de-serialized objects retrieved from the database.
  • Evil new Function() calls are slated to be eliminated by replacing view based queries with their equivalent GQL queries.
  • Models are required by Pouchdb to have _id and _rev properties. _id is unique within a database.
  • Models are required by the library to have type and name properties. Currently name is enforced to be unique within type. However, the name constraint is slated to be removed in favor of an available unique model validation, allowing any property to act as a unique key.
  • Model validation support will include all of validatejs's ActiveRecord style constraints, plus library specific unique, exists and notExists constraints.

Sample Code (implements the save/update operations):

// save or update an object, returns a promise -- exposed by the API
ret.saveObj = function (obj) {
    var err;

    var deferred = $q.defer();
    if (typeof obj === 'undefined') {
        err = {};
        err.message = "saveObj - Error: object undefined";
        safeApply($rootScope, function () {
            deferred.reject(err);
        });
    } else {
        save(obj, function (err, obj) {
            if (err) {
                safeApply($rootScope, function () {
                    deferred.reject(err);
                });
            } else {
                safeApply($rootScope, function () {
                    deferred.resolve(obj);
                });
            }
        });
    }
    return deferred.promise;
};

// ensure that Angularjs can 'see' any changes
var safeApply = function (scope, fn) {
    if (scope.$$phase || scope.$root.$$phase) {
        fn();
    } else {
        scope.$apply(fn);
    }
};

// private save / update code (async)
var save = function (obj, callback) {
    var err;
    var constraints = {
        type: {
            presence: true,
            simple: true,
            inclusion: {
                within: getNew
            }
        },
        name: {
            presence: true,
            simple: true
        }
    };
    err = validate(obj, constraints);
    if (typeof err !== 'undefined') {
        callback(err, obj);
    } else {
        // ensure name property is unique
        var body = '';
        if (obj._id && obj._id.length > 0) {
            body = "if(doc.type === '" + obj.type + "' && doc.name === '" + obj.name + "' && doc._id !== '" + obj._id + "') {emit(doc.name, doc);}";
        } else {
            body = "if(doc.type === '" + obj.type + "' && doc.name === '" + obj.name + "' ) {emit(doc.name, doc);}";
        }
        var view = {
            map: new Function("doc", body)
        };
        $db.query(view, {
            reduce: false
        }, function (error, response) {
            err = {};
            if (!error && response.rows.length > 0) {
                err.message = "Another object with the name '" + obj.name + "' already exists";
                callback(err, null);
            } else {
                // copy data to savable object      
                var serializableObj = {};
                for (var field in obj) {
                    if (obj.hasOwnProperty(field)) {
                        if (typeof obj[field] !== 'function') {
                            serializableObj[field] = angular.copy(obj[field]);
                        }
                    }
                }

                if (obj._rev !== '') {
                    $db.get(obj._id, function (err, doc) {
                        if (error) {
                            callback(error, null);
                        } else {
                            if (doc.type == obj.type) {
                                update(serializableObj, function (error, obj) {
                                    if (error) {
                                        callback(error, null);
                                    } else {
                                        obj._rev = serializableObj._rev;
                                        callback(null, obj);
                                    }
                                });
                            } else {
                                err = {};
                                err.message = "Cannot update object of type '" + doc.type + "' to type '" + obj.type + "'";
                                callback(err, null);
                            }
                        }
                    });
                } else {
                    insert(serializableObj, function (error, obj) {
                        if (error) {
                            callback(error, null);
                        } else {
                            obj._id = serializableObj._id;
                            obj._rev = serializableObj._rev;
                            callback(null, obj);
                        }
                    });
                }
            }
        });
    }
};

Tests (currently passing unless marked "PEND:"):

pouch-model
    delete expectations
        should delete an object by id
        should delete an object by object
        should return an error on delete by id if object does not exist
        should return an error on delete if object does not exist
        should return an error on delete by id if id is undefined
        should return an error on delete if object is undefined
        should return an error on delete if object is missing an id
    find expectations
        should get all of a single type
        should find an object by id and type
        should find an object by name and type
        should return null if object is not found
        should return an error if find by id not passed type or id
        should return an error if find by name not passed type or name
    prototype expectations
        should take object prototypes and return correct objects
        should return an error when attempting to create an object if type is not configured
        should return an error when attempting to find an object if type is not configured
        should return an error when attempting to get all if type is not configured
        should return an error when attempting to save objects if type is not configured
        should return an error when calling setNew() with a value that is not a function
    save expectations
        should save an object and then get it back
        should not save a new object with a duplicate name
        should save an object with a duplicate name if their type properties differ
        should return an error if name and type are not typeof string
        should return an error if name and type are empty strings
        should retain behavior after object is saved and restored from the pouchdb
        should restore model data recursively including array and object properties
        should return an error on missing name or type properties
        should return an error on missing undefined object
    update expectations
        should update an object and then get it back
        should not update an object to a duplicate name
        should update an object name to a duplicate name if types differ
        should return an error if type property is changed
    validation expectations
        PEND: should validate against pouch_model_validations if present
        PEND: should not return error when pouch_model_validations are not present
        PEND: should validate model for criteria unique on property
        PEND: should validate model for criteria unique on property within collection
        PEND: should validate model for criteria exists on type and property
        PEND: should validate model for criteria not exists on type and property
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The code looks good (make sure to replace new Function quickly :), and it's a great candidate for a public release.

It is short enough to deserve annotated documentation that would explain how it works which is useful for debugging. But that should not replace the actual API documentation.

\$\endgroup\$

Your Answer

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

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