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
asked Dec 11, 2013 at 20:06
\$\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.

answered Dec 13, 2013 at 12:38
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.