Code Guidelines
in [JavaScript]
|
Prev: Dynamic iframe caching problem workaround needed for Firefox
Next: JavaScript code mangler / scrambler / ... khm, more than obfuscator... :)
From: Garrett Smith on 21 Dec 2009 18:52 I'm putting together a couple of documents: 1) code guidelines 2) code review guidelines The goals are to help make for better code reviews here and to help debate on assessing javascript code quality. I'd like to start with my outline of code guidelines and get some feedback on it. Rich Internet Application Development Code Guildelines (Draft) Problems: Markup: * Invalid HTML * sending XHTML as text/html * xml prolog (cause problems in IE) * javascript: pseudo protocol * Not escaping ETAGO. Replace: "</" + "script>" with: "<\/script>"; Replace: "</td>"; with: "<\/td>"; CSS: * invalid css * classNames that do not have semantic meaning Script: Variables: * Global variables * Undeclared variables * non-descriptive name Methods: * initialization routines that loop through the DOM * methods that do too much or have side effects * long parameter lists * non-localized strings * inconsistent return types Methods should return one type; returning null instead of an Object may be a fair consideration. Strategies: * Modifying built-in or Host object in ways that are either error prone or confusing (LOD). * Use of non-standard or inconsistent ecmascript methods (substr, escape, parseInt with no radix). * Useless methods, e.g. goog.isDef = function(val) { return val !== undefined; }; Instead, replace useless method with typeof check: if(typeof val === "undefined"){ } Strings: * use of concatenation to repeatedly (repeatedly create and discard temporary strings) Instead use String.prototype.concat(a, b, c) or htmlBuf.push(a, b, c); Loops: * Loop body uses a long chain of identifiers Replace long chain of identifiers with variable. * Loop body traverses over elements to modify the style or event of each element. Solution: Styles: Replace a loop that applies styles to descendants by adding a class token to the nearest common ancestor and add a style rule to the style sheet with the selector text of #ancestor-id .descendant-class. Events: Use event delegation. Statements: * Use of == where === should be used * Boolean conversion of Host object (sometimes Error-prone) * Boolean conversion of value that may be falsish, e.g. if(e.pageX) * useless statements (e.g. typeof it == "array") RegularExpressions Be simple, but do not match the wrong thing. Formatting: * Tabs used instead of spaces. * Long lines DOM: * Use of poor inferences; browser detection * Use of non-standard approach or reliance on hacks * Branching for multiple conditions when common approach works in more browsers. * If an approach requires several conditions to branch, look for a different approach that works in all tested browsers. * Traversing the dom on page load (slow) especially traversing DOM using hand-rolled query selector. Instead of traversing the DOM, use Event delegation. Comments: * Inaccurate statement in comment * Comment doesn't match what code does -- Garrett comp.lang.javascript FAQ: http://jibbering.com/faq/
From: Erwin Moller on 22 Dec 2009 05:11 Garrett Smith schreef: > I'm putting together a couple of documents: > > 1) code guidelines > 2) code review guidelines > > The goals are to help make for better code reviews here and to help > debate on assessing javascript code quality. > > I'd like to start with my outline of code guidelines and get some > feedback on it. > <snipped> Hello Garrett, Sounds very interesting. I have read through it and I hope you will also give the reason for the do's and don'ts. Maybe with good examples and poor examples and some background information. That would be a great document for starters and JavaScript programmers with moderate experience. There is so much (poor) advice on the net when it comes to JavaScript, that is is often hard to find the right approach if you are not a specialist (in which case you don't need to search the web). Good luck! Regards, Erwin Moller -- "There are two ways of constructing a software design: One way is to make it so simple that there are obviously no deficiencies, and the other way is to make it so complicated that there are no obvious deficiencies. The first method is far more difficult." -- C.A.R. Hoare
From: Dmitry A. Soshnikov on 22 Dec 2009 05:55 On Dec 22, 2:52 am, Garrett Smith <dhtmlkitc...(a)gmail.com> wrote: Good parts, thanks. > > * inconsistent return types > Methods should return one type; returning null instead of an Object > may be a fair consideration. > Not necessarily. Method can be sort of factory, which returns different type values. Or, e.g. "analysis returns" when check is negative so simply exit from the function - there's no need to analyze this code more; in this case nor value neither its type is so essential (simple "return;" can be used without explicit value - implicit undefined), but maybe "return null;" will be better. From the other hand - why "return null" if the return statement at the end returns e.g. number (will you use "return -1" then)? // just exit, no need to analyze // code bellow if first check is not pass if (!someCheck) return; // other calculations return 100; > goog.isDef = function(val) { > return val !== undefined;}; > > Instead, replace useless method with typeof check: > if(typeof val === "undefined"){ } > From the speed point of view, calling function (establishing the new context and so on) sure is less effective. From the abstraction point of view, shorter wrapper can be better - with the same check inside - everyone chooses. I use typeof val === "undefined", but not against simple short wrappers. /ds
From: John G Harris on 22 Dec 2009 06:04 On Mon, 21 Dec 2009 at 15:52:55, in comp.lang.javascript, Garrett Smith wrote: <snip> >Formatting: > * Tabs used instead of spaces. Spaces are preferred, I hope. <snip> >Comments: > > * Inaccurate statement in comment > * Comment doesn't match what code does Avoid too many comments. (Let the code speak for itself). Remember to update comments when the code is changed. John -- John Harris
From: Asen Bozhilov on 22 Dec 2009 06:24
Garrett Smith wrote: > * Boolean conversion of Host object (sometimes Error-prone) //Boolean conversation host object in JScript var xhr = new ActiveXObject('Microsoft.XMLHTTP'); window.alert(Object.prototype.hasOwnProperty.call(xhr, 'open')); // true try { Boolean(xhr.open); }catch(e) { window.alert(e instanceof TypeError); //true window.alert(e); //Object doesn't support this property or method } window.alert(typeof xhr.open); //unknown |