From: Garrett Smith on
Thomas 'PointedEars' Lahn wrote:
> Garrett Smith wrote:
>
[...]

>> Instead, class and id should be meaningful, so the code is easier to
>> understand.
>
> Indubitably.
>
>> Bad:
>> .errorButton, #warningMessage
>>
>> Good:
>
> IBTD.
>
Good catch.

I put the "good" line in after "bad". Those are good examples.

I have the text in the draft:

Selectors such as .redButton, #ItalicStatement are meaningless.
Instead, use class and id that represent an object or state.
Example:

.errorButton, #warningMessage, .active-panel

This makes the code is easier to understand and doesn't
become meaningless when the styles change.

I am still replying to Stockton's comments on this, and it is a Holiday
here, so spending time with people now.
--
Garrett
comp.lang.javascript FAQ: http://jibbering.com/faq/
From: Garrett Smith on
Thomas 'PointedEars' Lahn wrote:
> Garrett Smith wrote:
>
>> Thomas 'PointedEars' Lahn wrote:
>>> Garrett Smith wrote:
>>>> Changed to:
>>>> * Use of == where strict equality is required
>>>> Where strict equality is required, the strict equality operator must be
>>>> used.
>>> That goes without saying.
>> It is a true statement, but not obvious to all.
>
> A spoon must be used where a spoon is required. How stupid does one have to
> be not to recognize the inherent veracity of such statements?
>

Not everyone knows the difference.

I have see a lot of code that uses == where === should have been used to
avoid the possibility of type conversion. When I see it, I point it out.

The mistake happens out of carelessness and inexperience.

I thought it would be useful to include this in the checklist, so that a
developer preparing the code for review could consider that.
--
Garrett
comp.lang.javascript FAQ: http://jibbering.com/faq/
From: Garrett Smith on
Dr J R Stockton wrote:
> In comp.lang.javascript message <hgp1oo$vee$1(a)news.eternal-
> september.org>, Mon, 21 Dec 2009 15:52:55, Garrett Smith
> <dhtmlkitchen(a)gmail.com> posted:
>
>> Rich Internet Application Development Code Guildelines (Draft)
>
> Remember the spelling/grammar checker.
>

Code Guidelines for Rich Internet Application Development

What do you think?

>
>> * non-localized strings
>
> You should mean non-internationalised strings, since that is a list of
> bad things.
>

I meant non-localized. For example, a problematic script that has:
showMessage("please enter your last name");

> Generally, one does better to list the good things.
>

The document is intended to be used primarily as a checklist for code
review preparation. Code must meet acceptibility criterion before going
into production, so that it can be determined if it is good or not.

It is foolish and omnipresent practice to ship messes to production.
This happens for many reasons, ignorance being the largest.

One such form of ignorance is the lack of criteria or standard by which
to assess code quality. This ignorance can be eliminated or at least
alleviated by providing a list of guidelines that defines code quality.

The code guidelines has been changed so that it is in the positive
sense, where appropriate, and negative sense, where appropriate. For
example:-

Markup:
* Use valid html.
* Use standards mode, with DOCTYPE first (no comment, xml prolog).
* Send html documents as text/html.
* Escape ETAGO with backwards solidus (backslash).
http://www.cs.tut.fi/~jkorpela/www/revsol.html
* Do not use javascript: pseudo protocol.

Sound better?

It may be worth considering assigning priority labels to certain things.
For example: "Use valid HTML" an "Use standards mode" would be priority
1 guidelines, while "Use efficient string concatenation techniques"
might be a Priority 2, while

>
>> * inconsistent return types
>> Methods should return one type; returning null instead of an Object
>> may be a fair consideration.
>
> Where successful operation of a routine cannot return anything
> resembling false, ISTM reasonable and useful to return something
> resembling false if the routine has failed to do what was wanted.
>

Functions that return multiple type provide more variance to the caller.

Sometimes this happens on accident, where the function was
intended to return one type, but can return a value of a different type.

Functions that return only one value type (including undefined) do not
burden the caller with having to check the return type.

It is something to look for in code where the function may, for example,
return a number in one case and a string in another case.

Functions that return multiple type have their place. A method might
return an object, but return null if the operation failed. So it is not
a rule, but something to consider when looking the code.

>
>> Strategies:
>> * Modifying built-in or Host object in ways that are either error
>> prone or confusing (LOD).
>
> Include : using without explanation jargon or any other than well-known
> acronyms & abbreviations
>
"Don't modify objects you don't own".

A piece of code that modifies objects it does not own creates a coupling
with those modifications and anything using that piece of code.

This coupling creates a dependency of those modifications to all the
dependencies of the code that exists in that codebase. Dependency of
those modifications is maximized to every piece of code that uses the
module that has modified them. Sometimes dependencies may even collide.
For example, one piece of code may create Element.prototype.hide, and
another piece of code may create a different Element.prototype.hide.

Instead, a separate interface should be created. The interface can be as
simple as a function. For example: hideElement( el ).

A fair exception to that rule would be for adding or fixing properties
that are either not yet implemented, or do not function as specified in
the standard.

if(!String.prototype.trim) {
// Patch for missing ES5 method.
}

An additional problem occurs modifying Host objects or Host objects'
constructors' prototypes. The outcome of this isn't specified and won't
work consistently cross-browser. The interface-based API design of the
w3c DOM allows for sub-interface to provide a more specific
implementation that shadows the user-defined method of the same name
further up in the prototype chain.

The w3c DOM does not prevent implementations from creating their own
interfaces. Quite often implementations do create interfaces that are
interwoven through the interface hierarchy. Such interfaces may or may
not be accessible to code.

>
>> 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);
>
> Remember that the chief use of JavaScript by the untended FAQ readership
> is to produce smallish pieces of code without much looping, running in a
> Web browser as a result of a user action. There is no point at all in
> telling a general FAQ reader that the less machine-efficient methods are
> wrong, if they work perfectly well and the code will complete within 100
> ms.
>

The code guidelines doc will help identify principles that make code
good and problems in code.

A motivation for this is to help facilitate better code reviews.

Some people would define good code as code that works, but that is a
very low standard.

Many recent posts on this list are a large dump of unformatted code,
followed by, and interspersed with, non-technical remarks and insults
directed towards the code author. Usually missing all or most of the
actual bugs in the process.

There is no document that defines *good* set of principles by which to
judge code.

I recently saw this entry on stack overflow:-
http://stackoverflow.com/questions/87896/code-reviews-on-the-web-for-php-and-javascript-code

| What are the best places for freelancers or small companies to get
| code reviewed for PHP and JavaScript? Forums are an option, but are
| there any sites dedicated specifically to code reviews?

There is a lot of advice out there already. Some online documents that I
have read web advocate principles without justification, make false
statements, and show bias by advocating a particular library.

> Writing the fastest code in a nice intellectual exercise for consenting
> participants, but it should not be imposed overall. The best code is
> easily-maintainable code, which generally means using a form of code
> which will be readily understandable to the author in three months time.
>
Code optimizations don't always reduce clarity. The best solutions are
simple and efficient. String.prototype.concat is simple and easy way to
avoid IE's slow string handling.

>
>> RegularExpressions
>> Be simple, but do not match the wrong thing.
>
> Generally - test not only that it works when it should, but also that it
> does not work when it should not.

That applies to functions, too.

> Include - don't repeat code where it can reasonably be avoided. Use
> temporary variables, or functions or methods, for this.
>
That's the basis for one of the DRY principle.

SOLID principles can also apply to js application architecture. SRP
applies to functions. LSP seems less closely related to js programming
where user-defined inheritance structures do not exist, though YUI panel
is an example of LSP violation.

SRP The Single Responsibility Principle
A class should have one, and only one, reason to change.
OCP The Open Closed Principle
You should be able to extend a classes behavior, without modifying it.
LSP The Liskov Substitution Principle
Derived classes must be substitutable for their base classes.
DIP The Dependency Inversion Principle
Depend on abstractions, not on concretions.
ISP The Interface Segregation Principle
Make fine grained interfaces that are client specific.
--
Garrett
comp.lang.javascript FAQ: http://jibbering.com/faq/
From: Eric Bednarz on
Garrett Smith <dhtmlkitchen(a)gmail.com> writes:

> * Use valid html.

If that really was a valid requirement for web authoring, it wouldn't be
joined by deep confusion about very basic and simple issues in almost
all cases.

> * Use standards mode, with DOCTYPE first (no comment, xml prolog).

Well, that didn't take long. The document type declaration *is part of
the prolog*, in both XML and SGML productions.
From: Garrett Smith on
Garrett Smith wrote:
> Dr J R Stockton wrote:
>> In comp.lang.javascript message <hgp1oo$vee$1(a)news.eternal-
>> september.org>, Mon, 21 Dec 2009 15:52:55, Garrett Smith
>> <dhtmlkitchen(a)gmail.com> posted:
>>
>>> Rich Internet Application Development Code Guildelines (Draft)
>>

[...]

> * Escape ETAGO with backwards solidus (backslash).
Changed in draft to "Escape ETAGO delimiter".
[...]
>
> An additional problem occurs modifying Host objects or Host objects'
> constructors' prototypes.

Should be "DOM objects' constructors' prototypes", to reflect ES5 change
to the definition of Host object, and the fact that a browser may
implement DOM object as Native object.

[...]
>
>> Include - don't repeat code where it can reasonably be avoided. Use
>> temporary variables, or functions or methods, for this.
>>
> That's the basis for one of the DRY principle.
>

correction: "for the DRY principle", not "for one of the DRY principle".
--
Garrett
comp.lang.javascript FAQ: http://jibbering.com/faq/