Jsperf Please Review Required Fields and Save Again
None of us write 100% issues-free code all of the time, and then don't feel there'southward a stigma attached to seeking help. Some of the virtually experienced developers in our industry, from framework authors to browser developers, regularly asking reviews of their lawmaking from others; asking whether something could be tweaked should in no style be considered embarrassing. Reviews are a technique like any other and should be used where possible.
Farther Reading on SmashingMag:
- JavaScript Mistakes To Avoid With A Static Code Analyzer
- Writing Fast, Memory-Efficient JavaScript
- JavaScript Profiling With The Chrome Developer Tools
- How To Proceed Your Coding Workflow Organized
Today we'll wait at where to go your code reviewed, how to structure your requests, and what reviewers look for. I was recently asked to review some code for a new JavaScript application, and thought I'd like to share some of my feedback, because information technology covers some JavaScript fundamentals that are ever useful to bear in mind.
More later bound! Proceed reading below ↓
Introduction
Reviewing code goes paw in hand with maintaining strong coding standards. That said, standards don't ordinarily preclude logical errors or misunderstandings well-nigh the quirks of a programming language, whether it'due south JavaScript, Ruby, Objective-C or something else. Even the near experienced developers can make these kinds of mistakes, and reviewing code can profoundly assist with communicable them.
The showtime reaction most of usa have to criticism is to defend ourselves (or our code), and possibly lash back. While criticism tin can be slightly demoralizing, recall of information technology as a learning feel that spurs u.s.a. to do meliorate and to better ourselves; considering in many cases, one time nosotros've calmed down, information technology actually does.
As well call up that no one is obliged to provide feedback on your work, and if the comments are indeed constructive, then be grateful for the time spent offer the input.
Reviews enable united states to build on the experience of others and to benefit from a 2nd pair of eyes. And at the terminate of the day, they are an opportunity for us to write amend code. Whether we take reward of them is entirely our choice.
Where Can I Get My Lawmaking Reviewed?
Often the most challenging part is actually finding an experienced programmer who yous trust to do the review. Beneath are some places where you tin asking others to review your code (sometimes in other languages, too).
- JSMentors JSMentors is a mailing list that discusses everything to practise with JavaScript (including Harmony), and a number of experienced developers are on its review panel (including JD Dalton, Angus Croll and Nicholas Zakas). These mentors might not always be readily available, but they practise their best to provide useful, constructive feedback on code that'due south been submitted. If you're looking for assistance with a specific JavaScript framework across vanilla JavaScript, the bulk of frameworks and libraries have mailing lists or forums that you can postal service to and that might provide a similar level of assistance.
- freenode IRC Many chat rooms here are defended both to discussing the JavaScript language and to requests for help or review. The near popular rooms are obviously named, and #javascript is particularly useful for generic JavaScript requests, while channels such as #jquery and #dojo are meliorate for questions and requests related to particular libraries and frameworks.
- Code Review (beta) You lot would be forgiven for confusing Code Review with StackOverflow, but it's actually a very useful, broad-spectrum, subjective tool for getting peer review of lawmaking. While on StackOverflow you might ask the question "Why isn't my code working?," Code Review is more suited to questions like "Why is my code and then ugly?" If yous all the same have any doubt about what information technology offers, I strongly recommend checking out the FAQs.
- Twitter This might sound odd, but at to the lowest degree half of the code that I submit for review is through social networks. Social networks work best, of course, if your code is open up source, but trying them never hurts. The only thing I suggest is to ensure that the developers who you follow and interact with are experienced; a review by a developer with insufficient experience tin sometimes be worse than no review at all, so be conscientious!
- GitHub + reviewth.is Nosotros all know that GitHub provides an fantabulous architecture for reviewing code. It comes with commits, file and line comments, update notifications, an easy way to track forks of gits and repositories, and more. All that's missing is a manner to actually initiate reviews. A tool called reviewth.is attempts to rectify that past giving you a post-commit claw that helps to automate this process, so changes that get posted in the wild take a clear #reviewthis hash tag, and you can tag any users who you wish to review your updates. If many of your colleagues happen to develop in the same language as you do, this set-up can work well for code reviews sourced closer to home. I workflow that works well with this (if you're working on a squad or on a collaborative project) is to perform your own work in a topic branch in a repository and so transport through pull requests on that branch. Reviewers would examine the changes and commits and could then make line-by-line and file-by-file comments. You (the programmer) would and so take this feedback and practise a destructive rebase on that topic co-operative, re-push it, and allow the review cycle to repeat until merging them would be acceptable.
How Should I Structure My Review Requests?
The following are some guidelines (based on experience) on how to construction your requests for code reviews, to increment the chances of them being accepted. You can exist more liberal with them if the reviewer is on your squad; but if the reviewer is external, and so these might salvage you lot some time:
- Isolate what you would like to be reviewed; ensure that it tin be easily run, forked and commented; be clear about where you remember improvements could exist made; and, above all, exist patient.
- Get in as like shooting fish in a barrel as possible for the reviewer to look at, demo and change your code.
- Don't submit a Zip file of your entire website or project; very few people have the time to become through all of this. The only state of affairs in which this would be acceptable is if your code admittedly required local testing.
- Instead, isolate and reduce what you would like to be reviewed on jsFiddle, on jsbin or in a GitHub gist. This will allow the reviewer to hands fork what you've provided and to show changes and comments on what can be improved. If you would prefer a "diff" between your work and any changes they've recommended, yous might also be interested in PasteBin, which supports this.
- Similarly, don't just submit a link to a page and ask them to "View source" in order to see what can be improved. On websites with a lot of scripts, this job would be challenging and lowers the chances of a reviewer like-minded to assist. No one wants to piece of work to find what you want reviewed.
- Clearly indicate where you lot personally feel the implementation could exist improved. This will help the reviewer quickly home in on what y'all're almost interested in having reviewed and will save them time. Many reviewers will still look at other parts of the code you lot've submitted regardless, merely at least help them prioritize.
- Indicate what (if whatsoever) research you lot've done into techniques for improving the lawmaking. The reviewer might very well suggest the same resources, merely if they're aware that yous already know of them, then they might offer alternative suggestions (which is what you desire).
- If English isn't your first language, there's no harm in saying so. When other developers inform me of this, I know whether to keep the language in my review technical or unproblematic.
- Be patient. Some reviews take several days to go back to me, and nothing'southward incorrect with that. Other developers are usually decorated with other projects, and someone who agrees to schedule a look at your work is being kind. Be patient, don't spam them with reminders, and be understanding if they get delayed. Doing this sometimes pay off, because the reviewer can provide even more detailed feedback when they take more fourth dimension.
What Should Code Reviews Provide?
Jonathan Betz, a former programmer at Google, once said that a lawmaking review should ideally accost 6 things:
- Correctness Does the code exercise everything it claims?
- Complexity Does information technology reach its goals in a straightforward way?
- Consistency Does it achieve its goals consistently?
- Maintainability Could the code be easily extended by another member of the team with a reasonable level of endeavor?
- Scalability Is the code written in such a way that it would work for both 100 users and x,000? Is it optimized?
- Style Does the code adhere to a item fashion guide (preferably i agreed upon by the team if the projection is collaborative)?
While I agree with this listing, expanding information technology into an action guide of what reviewers should practically aim to give developers would be useful. So, reviewers should do the post-obit:
- Provide articulate comments, demonstrate cognition, and communicate well.
- Signal out the shortfalls in an implementation (without being overly critical).
- State why a particular approach isn't recommended, and, if possible, refer to blog posts, gists, specifications, MDN pages and jsPerf tests to support the statement.
- Advise culling solutions, either in a divide runnable form or integrated in the code via a fork, so that the developer tin can clearly see what they did wrong.
- Focus on solutions first, and fashion 2nd. Suggestions on manner tin can come afterward in the review, merely address the fundamental problem as thoroughly as possible before paying attention to this.
- Review across the scope of what was requested. This is entirely at the reviewer's discretion, but if I notice problems with other aspects of a programmer's implementation, then I more often than not endeavor to advise them on how those, too, might be improved. I've yet to receive a complaint about this, so I assume information technology's not a bad affair.
Collaborative Code Reviews
Although a review past one developer tin work well, an alternative approach is to bring more than people into the procedure. This has a few distinct advantages, including reducing the load on individual reviewers and exposing more people to your implementation, which could potentially lead to more suggestions for improvements. It also allows a reviewer's comments to exist screened and corrected if they happen to make a mistake.
To assistance the group, you lot may wish to utilize a collaborative tool to allow all reviewers to simultaneously inspect and comment on your code. Luckily, a few decent ones out at that place are worth checking out:
- Review Board This Web-based tool is bachelor for gratuitous under the MIT license. Information technology integrates with Git, CVS, Mercurial, Subversion and a number of other source-command systems. Review Board can be installed on whatsoever server running Apache or lighttpd and is free for personal and commercial use.
- Crucible This tool by Australian software visitor Atlassian is too Web-based. It's aimed at the enterprise and works best with distributed teams. Crucible facilitates both alive review and alive commenting and, like Review Lath, integrates with a number of source-control tools, including Git and Subversion.
- Rietveld Like the other two, Rietveld likewise supports collaborative review, but it was actually written by the creator of Python, Guido van Rossum. Information technology is designed to run on Google'southward cloud service and benefits from Guido's experience writing Mondrian, the proprietary app that Google uses internally to review its lawmaking.
- Others A number of other options for collaborative code review weren't created for that purpose. These include CollabEdit (free and Web-based) and, my personal favorite, EtherPad (also free and Web-based).
Lessons From A JavaScript Code Review
On to the review.
A programmer recently wrote in, asking me to review their lawmaking and provide some useful suggestions on how they might amend information technology. While I'm certainly non an expert on reviewing code (don't allow the above fool you), here are the problems and solutions that I proposed.
Problem ane
Problem: Functions and objects are passed as arguments to other functions without any type validation.
Feedback: Type validation is an essential step in ensuring that you're working only with input of a desired type. Without sanitization checks in place, you run the gamble of users passing in just nearly anything (a string, a engagement, an assortment, etc.), which could easily break your awarding if y'all haven't developed it defensively. For functions, you lot should do the post-obit at a minimum:
- Examination to ensure that arguments being passed actually be,
- Do a
typeofcheque to forbid the app from executing input that is not a valid part at all.
if (callback && typeof callback === "function"){ /* residual of your logic */ }else{ /* not a valid part */ } Unfortunately, a elementary typeof check isn't enough on its ain. As Angus Croll points out in his mail service "Fixing the typeof operator," you demand to be aware of a number of issues with typeof checking if you're using them for anything other than functions.
For example, typeof goose egg returns object, which is technically incorrect. In fact, when typeof is applied to whatsoever object blazon that isn't a function, it returns object, not distinguishing between Assortment, Date, RegEx or whatever else.
The solution is to utilize Object.paradigm.toString to call the underlying internal property of JavaScript objects known as [[Class]], the grade property of the object. Unfortunately, specialized born objects generally overwrite Object.image.toString, but you can force the generic toString function on them:
Object.prototype.toString.call([i,2,three]); //"[object Array]" Yous might also find Angus's function below useful as a more reliable alternative to typeof. Try calling betterTypeOf() against objects, arrays and other types to see what happens.
function betterTypeOf( input ){ return Object.epitome.toString.call(input).friction match(/^[objects(.*)]$/)[1]; } Here, parseInt() is existence blindly used to parse an integer value of user input, but no base is specified. This tin can cause issues.
In JavaScript: The Good Parts, Douglas Crockford refers to parseInt() as being unsafe. Although y'all probably know that passing it a string argument returns an integer, you should also ideally specify a base or radix equally the 2d argument, otherwise information technology might return unexpected output. Take the following instance:
parseInt('20'); // returns what you expect, however… parseInt('020'); // returns 16 parseInt('000020'); // returns 16 parseInt('020', 10); // returns xx every bit nosotros've specified the base to utilise Yous'd be surprised by how many developers omit the second argument, but information technology happens quite regularly. Remember that your users (if permitted to freely enter numeric input) won't necessarily follow standard number conventions (because they're crazy!). I've seen 020, ,20, ;'20 and many other variations used, and so do your best to parse as broad a range of inputs as possible. The post-obit tricks to using parseInt() are occasionally amend:
Math.floor("020"); // returns 20 Math.floor("0020"); //returns 20 Number("020"); //returns 20 Number("0020"); //returns 20 +"020"; //returns 20 Problem 2
Problem: Checks for browser-specific conditions being met are repeated throughout the lawmaking base (for example, characteristic detection, checks for supported ES5 features, etc.).
Feedback: Ideally, your lawmaking base should be as DRY as possible, and there are some elegant solutions to this trouble. For instance, yous might benefit from the load-time configuration pattern here (too called load-time and init-time branching). The bones idea is that you examination a condition but once (when the awarding loads) and so access the upshot of that exam for all subsequent checks. This blueprint is usually found in JavaScript libraries that configure themselves at load time to be optimized for a particular browser.
This blueprint could exist implemented every bit follows:
var tools = { addMethod: null, removeMethod: nada }; if(/* status for native support */){ tools.addMethod = function(/* params */){ /* method logic */ } }else{ /* fallback - eg. for IE */ tools.addMethod = role(/* */){ /* method logic */ } } The example below demonstrates how this tin can exist used to normalize getting an XMLHttpRequest object.
var utils = { getXHR: cipher }; if(window.XMLHttpRequest){ utils.getXHR = part(){ return new XMLHttpRequest; } }else if(window.ActiveXObject){ utils.getXHR = role(){ /* this has been simplified for instance sakes */ return new ActiveXObject('Microsoft.XMLHTTP'); } } For a bully example, Stoyan Stefanov applies this to attaching and removing event listeners cross-browser, in his book JavaScript Patterns:
var utils = { addListener: goose egg, removeListener: null }; // the implementation if (typeof window.addEventListener === 'function') { utils.addListener = function ( el, type, fn ) { el.addEventListener(blazon, fn, false); }; utils.removeListener = function ( el, type, fn ) { el.removeEventListener(type, fn, false); }; } else if (typeof document.attachEvent === 'role') { // IE utils.addListener = office ( el, type, fn ) { el.attachEvent('on' + type, fn); }; utils.removeListener = function ( el, blazon, fn ) { el.detachEvent('on' + blazon, fn); }; } else { // older browsers utils.addListener = part ( el, blazon, fn ) { el['on' + type] = fn; }; utils.removeListener = function ( el, type, fn ) { el['on' + type] = nada; }; } Trouble 3
Trouble: The native Object.prototype is existence extended regularly.
Feedback: Extending native types is generally frowned upon, and few (if any) popular code bases should dare to extend Object.prototype. The reality is that there is not likely a situation in which you lot absolutely need to extend it in this way. In addition to breaking the object-every bit-hash tables in JavaScript and increasing the chance of naming collisions, it's more often than not considered bad practice, and modifying it should but exist a last resort (this is quite different from extending your ain custom object properties).
If for some reason you do end up extending the object epitome, ensure that the method doesn't already exist, and document information technology then that the rest of the team is aware why it'due south necessary. You can apply the following code sample as a guide:
if(typeof Object.epitome.myMethod != 'office'){ Object.prototype.myMethod = function(){ //implem }; } Juriy Zaytsev has a not bad post on extending native and host objects, which may be of interest.
Problem 4
Problem: Some of the code is heavily blocking the folio because it's either waiting on processes to complete or data to load earlier executing anything further.
Feedback: Page-blocking makes for a poor user feel, and in that location are a number of means to work around it without impairing the application.
Ane solution is to utilize "deferred execution" (via promises and futures). The basic idea with promises is that, rather than issuing blocking calls for resource, you immediately return a promise for a future value that will eventually exist fulfilled. This rather hands allows yous to write non-blocking logic that can exist run asynchronously. It is mutual to introduce callbacks into this equation that execute once the request completes.
I've written a relatively comprehensive post on this with Julian Aubourg, if you're interested in doing this through jQuery, but it tin of class exist implemented with vanilla JavaScript likewise.
Micro-framework Q offers a CommonJS-compatible implementation of promises and futures that is relatively comprehensive and can be used as follows:
/* define a promise-only delay part that resolves when a timeout completes */ function delay(ms) { var deferred = Q.defer(); setTimeout(deferred.resolve, ms); render deferred.promise; } /* usage of Q with the 'when' pattern to execute a callback once filibuster fulfils the hope */ Q.when(filibuster(500), function () { /* do stuff in the callback */ }); If you're looking for something more bones that can be read through, then hither is Douglas Crockford'due south implementation of promises:
function make_promise() { var status = 'unresolved', result, waiting = [], dreading = []; function vouch( deed, func ) { switch (condition) { case 'unresolved': (deed === 'fulfilled' ? waiting : dreading).push(func); break; case human action: func(consequence); break; } }; office resolve( deed, value ) { if (condition !== 'unresolved') { throw new Mistake('The hope has already been resolved:' + status); } condition = deed; effect = value; (deed == 'fulfilled' ? waiting : dreading).forEach(part (func) { endeavor { func(outcome); } catch (ignore) {} }); waiting = naught; dreading = cypher; }; return { when: part ( func ) { vouch('fulfilled', func); }, neglect: function ( func ) { vouch('smashed', func); }, fulfill: part ( value ) { resolve('fulfilled', value); }, nail: office ( string ) { resolve('smashed', string); }, condition: function () { render status; } }; }; Problem 5
Trouble: You're testing for explicit numeric equality of a property using the == operator, just you should probably be using === instead
Feedback: As yous may or may not know, the identity == operator in JavaScript is fairly liberal and considers values to exist equal fifty-fifty if they're of completely different types. This is due to the operator forcing a coercion of values into a single type (normally a number) prior to performing any comparison. The === operator will, however, not do this conversion, so if the two values beingness compared are non of the same type, then === will just return imitation.
The reason I recommend because === for more specific type comparing (in this case) is that == is known to have a number of gotchas and is considered to be unreliable by many developers.
You might also be interested to know that in abstractions of the linguistic communication, such as CoffeeScript, the == operator is completely dropped in favor of === beneath the hood due to the one-time'due south unreliability.
Rather than take my word for information technology, see the examples below of boolean checks for equality using ==, some of which result in rather unexpected outputs.
iii == "three" // true 3 == "03" // true 3 == "0003" // truthful 3 == "+iii" //true three == [three] //truthful 3 == (truthful+two) //true ' trn ' == 0 //true "trn" == 0 //true "t" == 0 // true "tn" == 0 // truthful "tr" == 0 // true " " == 0 // true " t" == 0 // truthful " " == 0 // true " rn " == 0 //true The reason that many of the (stranger) results in this listing evaluate to true is considering JavaScript is a weakly typed language: it applies type coercion wherever possible. If you're interested in learning more about why some of the expressions in a higher place evaluate to true, expect at the Annotated ES5 guide, whose explanations are rather fascinating.
Back to the review. If yous're 100% certain that the values being compared cannot be interfered with by the user, then go on with using the == operator with caution. Only recall that === covers your bases better in the event of an unexpected input.
Problem vi
Problem: An uncached array length is beingness used in all for loops. This is particularly bad because you're using it when iterating through an HTMLCollection.
Here's an example:
for( var i=0; i<myArray.length;i++ ){ /* practise stuff */ } Feedback: The problem with this approach (which I withal meet a number of developers using) is that the array length is unnecessarily re-accessed on each loop's iteration. This tin can exist very irksome, especially when working with HTMLCollections (in which case, caching the length can be anywhere upward to 190 times faster than repeatedly accessing it, every bit Nicholas C. Zakas mentions in his book High-Operation JavaScript). Below are some options for caching the assortment length.
/* cached outside loop */ var len = myArray.length; for ( var i = 0; i < len; i++ ) { } /* buried within loop */ for ( var i = 0, len = myArray.length; i < len; i++ ) { } /* cached outside loop using while */ var len = myArray.length; while (len--) { } A jsPerf test that compares the performance benefits of caching the array length inside and outside the loop, using prefix increments, counting downwards and more is as well available, if you would like to report which performs the best.
Problem 7
Problem: jQuery's $.each() is being used to iterate over objects and arrays, in some cases while for is beingness used in others.
Feedback: In jQuery, nosotros accept two ways to seamlessly iterate over objects and arrays. The generic $.each iterates over both of these types, whereas $.fn.each() iterates over a jQuery object specifically (where standard objects can be wrapped with $() should you lot wish to use them with the latter). While the lower-level $.each performs amend than $.fn.each(), both standard JavaScript for and while loops perform much better than either, as proven by this jsPerf examination. Beneath are some examples of loop alternatives that also perform improve:
/* jQuery $.each */ $.each(a, function() { eastward = $(this); }); /* classic for loop */ var len = a.length; for ( var i = 0; i < len; i++ ) { //if this must be a jQuery object do.. e = $(a[i]); //otherwise just due east = a[i] should suffice }; /* reverse for loop */ for ( var i = a.length; i-- ) { e = $(a[i]); } /* classic while loop */ var i = a.length; while (i--) { e = $(a[i]); } /* alternative while loop */ var i = a.length - 1; while ( e = a[i--] ) { $(e) }; Yous might find Angus Croll's post on "Rethinking JavaScript for Loops" an interesting extension to these suggestions.
Given that this is a information-axial application with a potentially big quantity of information in each object or assortment, you lot should consider a refactor to utilize i of these. From a scalability perspective, you want to shave off equally many milliseconds as possible from process-heavy routines, because these can build up when hundreds or thousands of elements are on the page.
Problem eight
Trouble: JSON strings are being congenital in-memory using cord concatenation.
Feedback: This could be approached in more than optimal ways. For instance, why not utilise JSON.stringify(), a method that accepts a JavaScript object and returns its JSON equivalent. Objects can generally be equally circuitous or as deeply nested as you wish, and this will nigh certainly result in a simpler, shorter solution.
var myData = {}; myData.dataA = ['a', 'b', 'c', 'd']; myData.dataB = { 'animal': 'cat', 'color': 'brown' }; myData.dataC = { 'vehicles': [{ 'blazon': 'ford', 'tint': 'silver', 'yr': '2015' }, { 'type': 'honda', 'tint': 'black', 'twelvemonth': '2012' }] }; myData.dataD = { 'buildings': [{ 'houses': [{ 'streetName': 'sycamore close', 'number': '252' }, { 'streetName': 'slimdon close', 'number': '101' }] }] }; console.log(myData); //object var jsonData = JSON.stringify(myData); console.log(jsonData); /* {"dataA":["a","b","c","d"],"dataB":{"animal":"cat","color":"chocolate-brown"},"dataC":{"vehicles":[{"type":"ford","tint":"silverish","year":"2015"},{"type":"honda","tint":"blackness","year":"2012"}]},"dataD":{"buildings":[{"houses":[{"streetName":"sycamore close","number":"252"},{"streetName":"slimdon close","number":"101"}]}]}} */ Equally an extra debugging tip, if yous would similar to pretty-print JSON in your console for easier reading, then the following actress arguments to stringify() will accomplish this:
JSON.stringify({ foo: "how-do-you-do", bar: "earth" }, null, 4); Problem 9
Problem: The namespacing pattern used is technically invalid.
Feedback: While namespacing is implemented correctly across the rest of the application, the initial bank check for namespace existence is invalid. Hither's what you lot currently have:
if ( !MyNamespace ) { MyNamespace = { }; } The trouble is that !MyNamespace will throw a ReferenceError, considering the MyNamespace variable was never declared. A meliorate pattern would have advantage of boolean conversion with an inner variable declaration, every bit follows:
if ( !MyNamespace ) { var MyNamespace = { }; } Problem: Some of the code is heavily blocking the page because information technology's either waiting on processes to complete or data to load before executing anything further.
Feedback: Page-blocking makes for a poor user experience, and at that place are a number of ways to work around it without impairing the application.
One solution is to use "deferred execution" (via promises and futures). The basic idea with promises is that, rather than issuing blocking calls for resources, you lot immediately return a promise for a futurity value that will eventually be fulfilled. This rather hands allows you to write non-blocking logic that tin be run asynchronously. It is common to introduce callbacks into this equation that execute one time the asking completes.
I've written a relatively comprehensive post on this with Julian Aubourg, if you lot're interested in doing this through jQuery, but it can of course be implemented with vanilla JavaScript as well.
Micro-framework Q offers a CommonJS-compatible implementation of promises and futures that is relatively comprehensive and can be used as follows:
/* define a hope-merely filibuster function that resolves when a timeout completes */ function delay(ms) { var deferred = Q.defer(); setTimeout(deferred.resolve, ms); render deferred.promise; } /* usage of Q with the 'when' pattern to execute a callback once delay fulfils the promise */ Q.when(delay(500), part () { /* do stuff in the callback */ }); If y'all're looking for something more basic that can be read through, and then hither is Douglas Crockford's implementation of promises:
function make_promise() { var status = 'unresolved', outcome, waiting = [], dreading = []; part vouch( deed, func ) { switch (status) { instance 'unresolved': (deed === 'fulfilled' ? waiting : dreading).push(func); suspension; case deed: func(effect); break; } }; function resolve( deed, value ) { if (status !== 'unresolved') { throw new Error('The promise has already been resolved:' + status); } status = deed; result = value; (deed == 'fulfilled' ? waiting : dreading).forEach(function (func) { effort { func(outcome); } catch (ignore) {} }); waiting = nil; dreading = null; }; return { when: function ( func ) { vouch('fulfilled', func); }, fail: function ( func ) { vouch('smashed', func); }, fulfill: function ( value ) { resolve('fulfilled', value); }, boom: function ( string ) { resolve('smashed', string); }, status: function () { return status; } }; }; Problem 5
Problem: Y'all're testing for explicit numeric equality of a property using the == operator, but you lot should probably be using === instead
Feedback: As yous may or may not know, the identity == operator in JavaScript is fairly liberal and considers values to be equal even if they're of completely different types. This is due to the operator forcing a coercion of values into a single blazon (usually a number) prior to performing any comparison. The === operator will, nevertheless, non practise this conversion, so if the ii values existence compared are not of the aforementioned blazon, then === will just return false.
The reason I recommend considering === for more specific blazon comparison (in this case) is that == is known to have a number of gotchas and is considered to be unreliable by many developers.
You might besides be interested to know that in abstractions of the linguistic communication, such equally CoffeeScript, the == operator is completely dropped in favor of === beneath the hood due to the former's unreliability.
Rather than take my word for information technology, see the examples below of boolean checks for equality using ==, some of which outcome in rather unexpected outputs.
iii == "three" // true 3 == "03" // true three == "0003" // true iii == "+3" //true 3 == [3] //true three == (true+2) //true ' trn ' == 0 //truthful "trn" == 0 //true "t" == 0 // truthful "tn" == 0 // true "tr" == 0 // true " " == 0 // true " t" == 0 // true " " == 0 // true " rn " == 0 //truthful The reason that many of the (stranger) results in this listing evaluate to true is because JavaScript is a weakly typed language: information technology applies type coercion wherever possible. If you lot're interested in learning more nearly why some of the expressions above evaluate to true, look at the Annotated ES5 guide, whose explanations are rather fascinating.
Back to the review. If you're 100% certain that the values existence compared cannot be interfered with by the user, then proceed with using the == operator with circumspection. Just recollect that === covers your bases ameliorate in the outcome of an unexpected input.
Problem half dozen
Problem: An uncached array length is being used in all for loops. This is particularly bad because y'all're using it when iterating through an HTMLCollection.
Here's an example:
for( var i=0; i<myArray.length;i++ ){ /* practise stuff */ } Feedback: The trouble with this approach (which I still run into a number of developers using) is that the array length is unnecessarily re-accessed on each loop's iteration. This can be very slow, peculiarly when working with HTMLCollections (in which case, caching the length tin exist anywhere upwardly to 190 times faster than repeatedly accessing it, every bit Nicholas C. Zakas mentions in his book High-Operation JavaScript). Below are some options for caching the array length.
/* buried outside loop */ var len = myArray.length; for ( var i = 0; i < len; i++ ) { } /* cached within loop */ for ( var i = 0, len = myArray.length; i < len; i++ ) { } /* cached exterior loop using while */ var len = myArray.length; while (len--) { } A jsPerf test that compares the performance benefits of caching the array length within and exterior the loop, using prefix increments, counting downwardly and more is too available, if you would like to written report which performs the best.
Problem 7
Problem: jQuery's $.each() is beingness used to iterate over objects and arrays, in some cases while for is being used in others.
Feedback: In jQuery, nosotros have ii ways to seamlessly iterate over objects and arrays. The generic $.each iterates over both of these types, whereas $.fn.each() iterates over a jQuery object specifically (where standard objects can be wrapped with $() should y'all wish to employ them with the latter). While the lower-level $.each performs better than $.fn.each(), both standard JavaScript for and while loops perform much improve than either, every bit proven past this jsPerf examination. Beneath are some examples of loop alternatives that also perform better:
/* jQuery $.each */ $.each(a, function() { e = $(this); }); /* archetype for loop */ var len = a.length; for ( var i = 0; i < len; i++ ) { //if this must be a jQuery object do.. e = $(a[i]); //otherwise merely eastward = a[i] should suffice }; /* opposite for loop */ for ( var i = a.length; i-- ) { due east = $(a[i]); } /* classic while loop */ var i = a.length; while (i--) { e = $(a[i]); } /* alternative while loop */ var i = a.length - i; while ( east = a[i--] ) { $(e) }; Yous might notice Angus Croll's mail on "Rethinking JavaScript for Loops" an interesting extension to these suggestions.
Given that this is a data-axial application with a potentially big quantity of data in each object or array, you should consider a refactor to apply i of these. From a scalability perspective, you lot desire to shave off as many milliseconds equally possible from procedure-heavy routines, because these can build up when hundreds or thousands of elements are on the page.
Problem 8
Trouble: JSON strings are being built in-memory using string concatenation.
Feedback: This could be approached in more optimal ways. For instance, why not use JSON.stringify(), a method that accepts a JavaScript object and returns its JSON equivalent. Objects can generally be as complex or as deeply nested as you wish, and this volition almost certainly issue in a simpler, shorter solution.
var myData = {}; myData.dataA = ['a', 'b', 'c', 'd']; myData.dataB = { 'animal': 'cat', 'color': 'brown' }; myData.dataC = { 'vehicles': [{ 'type': 'ford', 'tint': 'silver', 'year': '2015' }, { 'type': 'honda', 'tint': 'blackness', 'year': '2012' }] }; myData.dataD = { 'buildings': [{ 'houses': [{ 'streetName': 'sycamore close', 'number': '252' }, { 'streetName': 'slimdon close', 'number': '101' }] }] }; console.log(myData); //object var jsonData = JSON.stringify(myData); console.log(jsonData); /* {"dataA":["a","b","c","d"],"dataB":{"animal":"cat","color":"brown"},"dataC":{"vehicles":[{"blazon":"ford","tint":"silver","yr":"2015"},{"type":"honda","tint":"black","year":"2012"}]},"dataD":{"buildings":[{"houses":[{"streetName":"sycamore close","number":"252"},{"streetName":"slimdon shut","number":"101"}]}]}} */ As an actress debugging tip, if you would like to pretty-print JSON in your console for easier reading, and so the following extra arguments to stringify() will accomplish this:
JSON.stringify({ foo: "hello", bar: "world" }, null, iv); Problem nine
Trouble: The namespacing pattern used is technically invalid.
Feedback: While namespacing is implemented correctly across the residuum of the application, the initial check for namespace existence is invalid. Here's what you currently have:
if ( !MyNamespace ) { MyNamespace = { }; } The problem is that !MyNamespace will throw a ReferenceError, because the MyNamespace variable was never declared. A meliorate pattern would take advantage of boolean conversion with an inner variable declaration, as follows:
if ( !MyNamespace ) { var MyNamespace = { }; } //or var myNamespace = myNamespace || {}; // Although a more efficient way of doing this is: // myNamespace || ( myNamespace = {} ); // jsPerf test: https://jsperf.com/conditional-assignment //or if ( typeof MyNamespace == 'undefined' ) { var MyNamespace = { }; } This could, of course, be washed in numerous other ways. If you're interested in reading most more namespacing patterns (as well as some ideas on namespace extension), I recently wrote "Essential JavaScript Namespacing Patterns." Juriy Zaytsev too has a pretty comprehensive post on namespacing patterns.
Decision
That's it. Reviewing lawmaking is a great mode to enforce and maintain quality, correctness and consistency in coding standards at as high a level every bit possible. I strongly recommend that all developers give them a effort in their daily projects, considering they're an fantabulous learning tool for both the developer and the reviewer. Until next time, effort getting your code reviewed, and adept luck with the rest of your project!
(al, il)
Source: https://www.smashingmagazine.com/2011/10/lessons-from-a-review-of-javascript-code/
0 Response to "Jsperf Please Review Required Fields and Save Again"
Post a Comment