Petar Slovic
JSPolice Review & Contributions
Let's improve JS Open Source Projects.
Contribute!

ReadRemaining

Petar Slovic Petar Slovic

Repo: https://github.com/Aerolab/readremaining.js

ReadRemaining is a cool jQuery plugin that calculates the time needed to read a text on a web page, and then as you scroll, it updates that info, telling you how long until the end of text, based on your scrolling speed.

Read Remaining in action

Code

Code looks good. There are classic patterns for defining jQuery plugins at work. Wrapping the code with

;(function($, window, document, undefined) {
  // code
})(jQuery, window, document);

has several benefits. The ; at the begining of the line prevents errors that can happen due to concatenation of files, and some of them not having the closing ;.

Wrapping the code in an IIFE - Immediately Invoked Funciton Expression creates an isolated local scope, that the rest of the code cannot touch, without us explicitly allowing that.

Lastly, passing the global arguments into our IIFE does a few things. First, it reduces the lookups up the prototype chain, giving us a tiny performance optimization. But, more importantly, by explicitly passing global arguments to our code like this, we avoid name confilcts. For example, by using $ in our code, we might get into trouble with other libraries also using that symbol, but when we pass jQuery when invoking the IIFE and use it as $ inside it, we know that we’re referring to jQuery. Lastly, by accepting undefined as the last argument, we guard ourselves against somebody maliciously overriding it.

Plugin Registration

The plugin is implemented as a function, with methods added it its prototype, which is a implementation of a class in JavaScript. That’s great. Also, there’s a specific way of registering the plugin with jQuery, which guards against multiple instances.

/* jQuery Plugin Boilerplate: A really lightweight plugin wrapper around the constructor, preventing against multiple instantiations */  
  $.fn[pluginName]=function(options){if(typeof arguments[0]==="string"){var methodName=arguments[0];var args=Array.prototype.slice.call(arguments,1);var returnVal;this.each(function(){if($.data(this,"plugin_"+pluginName)&&typeof $.data(this,"plugin_"+pluginName)[methodName]==="function")returnVal=$.data(this,"plugin_"+pluginName)[methodName].apply(this,args);else throw new Error("Method "+methodName+" does not exist on jQuery."+pluginName);});if(returnVal!==undefined)return returnVal;else return this}else if(typeof options==="object"||!options)return this.each(function(){if(!$.data(this,"plugin_"+pluginName))$.data(this,"plugin_"+pluginName,new Plugin(this,options))});return this};

Plugin Options

There are a couple of ways to implement options in a plugin. Most often we allow the user to pass a configuration object, containing all the custom options, and we have our defaults defined in the plugin, and then we merge the two objects, therefore user’s configuration overwriting our defaults.

In this plugin we see that approach, with $.extend method used to merge the options that user passed with the default ones.

var defaults = {
  showGaugeDelay   : 1000,
  showGaugeOnStart : false,
  timeFormat       : '%mm %ss left',
  maxTimeToShow    : 20*60,
  minTimeToShow    : 10,
  gaugeContainer   : '',
  insertPosition   : 'prepend',
  verboseMode      : false,
  gaugeWrapper     : '',
  topOffset        : 0,
  bottomOffset     : 0
};

this.settings = $.extend({}, defaults, options);

Code Style

One thing I didn’t like are inconsistencies in code style.

Sometimes you’ll find single quotes, and sometimes double quotes. Sometimes an if statement will not have surronding brackets. == is used in favour of ===.

Comments

There’s a lot of comments, and they contain useful information.

At the top, there are comments labeled by 1., 1.1, 1.2… and used in code by referencing that label. I’m not sure if I like this. It is an interesting idea, but it may make the reading of code harder, since you have to scroll to the top every time you encounter a label as a comment.

Contributions

We’ve sent a few pull requests, addressing some of the things we found are missing:

This plguin is a great idea and a really good execution. Most of the important things from the checklist are there. A few things are missing though:

Atomic commits

There is just one big commit with most of the code, and then a few others that fix typos or add css. We should commit atomically so that other people can see how the code evolved and got to the point where it is now. So in the future, it would be great to see some commits that are better organized.

Tests

There are no tests included. This plugin could be tested nicely, checking the consistency and results of the function that figures out how much time will reading a text (or a part of it) take. Testing scrolling behavior might be a bit tricky (http://spirytoos.blogspot.rs/2014/02/testing-windowscroll-with-qunitjasmine.html).

Linter

We should have a JavaScript linter in the project, so that we can be notified when we’re not writing consistently styled code.

Unclosed issues

There are several issues that aren’t closed. Even though the author did discuss the course of action in most of the issues, no action was taken.

Full Checklist

GitHub Page / Docs:

Issues & Collaboration

Git

The Code