Jump to: navigation, search

Difference between revisions of "Horizon/Javascript"

(Required:)
(Required:)
Line 8: Line 8:
 
=== JavaScript ===
 
=== JavaScript ===
 
==== Required: ====
 
==== Required: ====
To start out with here is a checklist of "things not to do".
+
We listed out some 'do's' and 'don't do's' in JavaScript.  Hopefully this will be a quick reference for you to check your code against.
 +
 
 +
 
 +
'''Code Review Do Checklist'''  -
 +
* Give meaning full names to methods and variables.   
 +
* Put a comment at top of every file explaining what the purpose of this class is.  This applies to methods and variables when the naming is not obvious.     
 +
*  Use '===' as oppose to '==' for equality checks.  There are two classes of comparison operators.  The '==' will do a type cast before comparing, which can lead to unwanted results.       
 +
* Keep DOM access to a minimum.    It can be expensive, and can become a performance issue.  Make sure that if you are accessing the DOM that you are doing it in the most optimized way.  For example build up document fragments and append once to the DOM vs smaller multiple updates.
 +
* Enclose each JS file inside of a self-executing function. Then use "strict". Like this: (function(){ 'use strict'; ...  The self-executing function keeps the strict scoped to the file, so it does not apply to other JS files in the build.    Also throws exceptions if you code is trying to access global variables.
  
 
'''Code Review Don't Do Checklist''' -
 
'''Code Review Don't Do Checklist''' -
Line 20: Line 28:
 
* No commented-out code.
 
* No commented-out code.
 
* Remove dead code
 
* Remove dead code
 
 
'''Code Review Do Checklist'''  -
 
* Give meaning full names to methods and variables.   
 
* Put a comment at top of every file explaining what this class is for.  This applies to methods and variables when meaning is not obvious.     
 
*  Use '===' as oppose to '==' for equality checks.  There are two classes of comparison operators.  The '==' with type cast before comparing, which can lead to unwanted results.       
 
* Keep DOM manipulations to a minimum.    They can be expensive, and can become a performance issue.   
 
* Enclose each JS file inside of a self-executing function. Then use "strict". Like this: (function(){ 'use strict'; ...  The self-executing function keeps the strict scoped to the file, so it does not apply to other JS files in the build.    Also throws exceptions if you code is trying to access global variables.
 
  
 
Keeping the above checklist's in mind is good practice, but what if you can't find the URL or just forget to check for one of the practices.  That is where JSHint comes in.
 
Keeping the above checklist's in mind is good practice, but what if you can't find the URL or just forget to check for one of the practices.  That is where JSHint comes in.

Revision as of 19:30, 13 August 2014

****** Under Construction ******

Code Style Guidelines

As a project, Horizon adheres to code quality standards. To that end we listed out coding standards and best practices for the different languages divided into Required, Recommended and Optional sections. Our main goal in establishing these best practices is to have code that is readable, maintainable, and reliable.

JavaScript

Required:

We listed out some 'do's' and 'don't do's' in JavaScript. Hopefully this will be a quick reference for you to check your code against.


Code Review Do Checklist -

  • Give meaning full names to methods and variables.
  • Put a comment at top of every file explaining what the purpose of this class is. This applies to methods and variables when the naming is not obvious.
  • Use '===' as oppose to '==' for equality checks. There are two classes of comparison operators. The '==' will do a type cast before comparing, which can lead to unwanted results.
  • Keep DOM access to a minimum. It can be expensive, and can become a performance issue. Make sure that if you are accessing the DOM that you are doing it in the most optimized way. For example build up document fragments and append once to the DOM vs smaller multiple updates.
  • Enclose each JS file inside of a self-executing function. Then use "strict". Like this: (function(){ 'use strict'; ... The self-executing function keeps the strict scoped to the file, so it does not apply to other JS files in the build. Also throws exceptions if you code is trying to access global variables.

Code Review Don't Do Checklist -

  • No variables or functions in the global namespace. There are several reasons, why globals are bad, one being that all JavaScript included in an application runs in the same scope. Which means that if another script has the same method or variable names they overwrite each.
  • Always use var for variables unless its intended for global space. But as mentioned above, global variables in general are bad, so unless you have specific reason to use them look for alternatives.
  • Avoid using eval( ). The eval () just does that, it evaluates the expression passed to it. This can open up your code to security vulnerabilities, and other issues. I know some will argue that using eval is not always dangerous, but this is a best practice document after all. In most cases there is a viable alternative.
  • Avoid using with(). The with() method is used access properties of an object. The issue with 'with' is that its execution is not consistent, so by reading the statement it is not clear how it is being used.
  • Avoid excessive nesting. The rest of the items in the "don't" list is to keep the code more reader friendly and maintainable (and are self explanatory).
  • Avoid excessive function chaining.
  • Avoid HTML and CSS in JS code; use templates and stylesheets
  • No commented-out code.
  • Remove dead code

Keeping the above checklist's in mind is good practice, but what if you can't find the URL or just forget to check for one of the practices. That is where JSHint comes in.

JSHint improves JavaScript quality by checking your code against a configurable list of checks. Therefore, JavaScript developers should configure their editors to use JSHint to warn them of any errors. Horizon does not include JSHint in the build for legal reasons, so it is only used during code editing. Instructions for setting up JSHint for Eclipse, Sublime Text, Notepad++ and WebStorm/PyCharm are provided.

Recommended:

Source-code formatting – (or “beautification”) is recommend but should be used with caution. Keep in mind that if you reformat an entire file that was not previously formatted the same way, it will mess up the diff during the code review. It is best to use a formatter when you are working on a new file by yourself, or with others who are using the same formatter. You can also choose to format a selected portion of a file only. Instructions for setting up JSHint for Eclipse, Sublime Text, Notepad++ and WebStorm/PyCharm are provided.

Optional:

AngularJS

Define your angular app under the angular folder (such as horizon.table.js). You can choose to lump them all in one file. If your file is getting too big and readability becomes an issue, consider moving them into the individual folders (filters, directives, etc..)

Separate presentation and business logic. Controllers and Services should not contain DOM references. Directives do. Controllers have view behaviors Services are singletons and contain logic independent of view

Scope - Read-only in from templates, write-only in controllers

Required:

For testability, do not put view logic inside controllers or filters. Put it in directives when necessary.

Recommended:

Since Django already uses {{ }}, use {$ $} instead or {% verbatim %}

Don't use variables like "app" that are at the highest level in the file, when Angular gives an alternative, instead you function chaining.

Put "Ctrl" on the end of a controller name Use these directories: filters, directives, controllers, and templates. When you use the directory name, the file name does not have to include words like "directive" or "filter".

Optional: