Skip to content

Commit

Permalink
OLMIS-5815: Update coding conventions
Browse files Browse the repository at this point in the history
* Added information about Class patter
* Added information about classExtender
* Fixed some typos
  • Loading branch information
ngraczewski committed Dec 12, 2018
1 parent a70b9b7 commit ca45fea
Show file tree
Hide file tree
Showing 8 changed files with 238 additions and 72 deletions.
18 changes: 10 additions & 8 deletions docs/conventions-angularjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,32 +42,32 @@ that the controller exposes.

```
/**
* @ngdoc service
* @ngdoc controller
* @name module-name.controller:controllerName
*
* @description
* Controller description.
*
*/
```


## Directive
Directives are pieces of HTML markup that have been extended to do a certain function. *This is the only place where it is reasonable to manipulate the DOM*.

*Make disticntion between directive and component -- components use E tag and isolate scope, directive use C and never isolate scope*
*Make distinction between directive and component -- components use E tag and isolate scope, directive use C and never isolate scope*

### Conventions
* Restrict directives to only elements or attributes
* Don't use an isolated scope unless you absolutely have to
* If the directive needs extenal information, use a controller — don't manipulate data in a link function
* If the directive needs external information, use a controller — don't manipulate data in a link function

### Unit Testing
The bit secrect when unit testing a directive is to make sure to use the $compile function to return an element that is extended with jQuery. Once you have this object you will be able to interact with the directive by clicking, hovering, or triggering other DOM events.
The bit secret when unit testing a directive is to make sure to use the $compile function to return an element that is extended with jQuery. Once you have this object you will be able to interact with the directive by clicking, hovering, or triggering other DOM events.

```
describe('SampleDirective', function(){
it('gets compiled and shows the selected item name', function($compile, $rootScope){
describe('SampleDirective', function() {
it('gets compiled and shows the selected item name', function($compile, $rootScope) {
var scope = $rootScope.$new();
scope['item'] = {
name: "Sample Title"
Expand All @@ -76,7 +76,8 @@ describe('SampleDirective', function(){
expect(element.text()).toBe("Sample Title");
});
it('responds to being clicked', function($compile, $rootScope){
it('responds to being clicked', function($compile, $rootScope) {
var element = $compile("<sample-directive selected='item'></sample-directive>")($rootScope.$new());
// check before the action
Expand All @@ -87,6 +88,7 @@ describe('SampleDirective', function(){
// this could also be looking at a spy to see what the values are
expect(element.text()).toBe("I was clicked");
});
});
```

Expand Down
249 changes: 203 additions & 46 deletions docs/conventions-javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ This document describes the desired formatting to be used within the OpenLMIS-UI

## General
The following conventions should be applied to all sections of UI development:
* All intentation should be 4 spaces
* All indentation should be 4 spaces
* Legacy code should be refactored to meet coding conventions
* No thrid party libraries should be included in a OpenLMIS-UI repository
* No third party libraries should be included in a OpenLMIS-UI repository

## File Structure
All file types should be organized together within the `src` directory according to functionality, not file type — the goal is to keep related files together.

Use the following conventions:
* File names are lowercase and dash-seperated
* File names are lowercase and dash-separated
* Files in a directory should be as flat as possible (avoid sub-directories)
* If there are more than 12 files in a directory, try to divide files into subdirectories based on functional area

Expand All @@ -24,16 +24,16 @@ Generally, all file names should use the following format `specific-name.file-ty
* `ext` is the extension of the file (ie '.js', '.scss')

Folder structure should aim to follow the [LIFT principal](https://github.com/johnpapa/angular-styleguide/tree/master/a1#application-structure-lift-principle) as closely as possible, with a couple extra notes:
* There should only be one *.module.js file per directory hiearchy
* Only consider creating a sub-directory if file names are long and repatitive, such that a sub-directory would improve meaning
* There should only be one *.module.js file per directory hierarchy
* Only consider creating a sub-directory if file names are long and repetitive, such that a sub-directory would improve meaning
*Each file type section below has specifics on their naming conventions*


## Javascript Guidelines
Almost everything in the OpenLMIS-UI is Javascript. These are general guidelines for how to write and test your code.

General conventions:
* All code should be within an [immedately invoked scope](https://github.com/johnpapa/angular-styleguide/tree/master/a1#iife)
* All code should be within an [immediately invoked scope](https://github.com/johnpapa/angular-styleguide/tree/master/a1#iife)
* *ONLY ONE OBJECT PER FILE*
* Variable and function names should be written in camelCase
* All Angular object names should be written in CamelCase
Expand Down Expand Up @@ -112,66 +112,32 @@ Properties should have parameters like in the following example:


## Constants
Constants are Javascript variables that won't change but need to be resued between multiple objects within an Angular module. Using constants is important because it becomes possible to track an objects dependencies, rather than use variables set on the global scope.
Constants are Javascript variables that won't change but need to be reused between multiple objects within an Angular module. Using constants is important because it becomes possible to track an objects dependencies, rather than use variables set on the global scope.

It's also [useful to wrap 3rd party objects and libraries](https://github.com/johnpapa/angular-styleguide/blob/master/a1/README.md#vendor-globals) (like jQuery or bootbox) as an Angular constant. This is useful because the dependency is declared on the object. Another useful feature is that if the library or object isn't included, Angualr will throw a single verbose error message.
It's also [useful to wrap 3rd party objects and libraries](https://github.com/johnpapa/angular-styleguide/blob/master/a1/README.md#vendor-globals) (like jQuery or bootbox) as an Angular constant. This is useful because the dependency is declared on the object. Another useful feature is that if the library or object isn't included, Angular will throw a single verbose error message.

*Add rule about when its ok to add a group of constants -- if a grouping of values, use a plural name*

*Conventions:*
* All constant variable names should be upper case and use underscores instead of spaces (ie VARIABLE_NAME)
* If a constant is only relivant to a single Angular object, set it as a variable inside the scope, not as an Angular constant
* If a constant is only relevant to a single Angular object, set it as a variable inside the scope, not as an Angular constant
* If the constant value needs to change depending on build variables, format the value like @@VARIABLE_VALUE, and which should be replaced by the grunt build process if there is a matching value
* Wrap 3rd party services as constants, if are not already registered with Angular

### Replaced Values
@@ should set own default values

## Factory
Factories should be the most used Angular object type in any application. [John Papa insists that factories serve a single purpose](https://github.com/johnpapa/angular-styleguide/blob/master/a1/README.md#factories) and should be extended by variabled they are called with.

This means that factories should generally return a function that will return an object or set of objects that can be manipulated. It is common for a factory to include methods for interacting with a server, but this isn't necessary.

_Should be used with UI-Router resolves, and get additional arguments_

### Naming Convention
_**specificName**Factory_

Factories should always be named lowercase camelCase. To avoid confussion between created objects and factories, all factories should have the word'Factory' appended to the end (this disagrees with John-Papa style).

### Example

```
angular.module('openlmis-sample')
.factory('sampleFactory', sample);
sample.$inject = [];
function sample(){
var savedContext;
return {
method: method,
otherMethod: otherMethod
}
}
```

*Unit Testing Conventions*
Test a factory much like you would test a service, except be sure to:
* Declare a new factory at the start of every test
* Exercise the produced object, not just the callback function

## Interceptor
This section is about events and messages, and how to modify them.

HTTP Interceptors are technically factories that have been configured to 'intercept' certain types of requests in Angular and modify their behavior. This is recommended because other Angular objects can use consistent Angular objects, reducing the need to write code that is specialized for our own framework.

*Keep all objects in a single file - so its easier to understand the actions that are being taken*

The Angular guide to writting [HTTP Interceptors is here](https://docs.angularjs.org/api/ng/service/$http#interceptors)
The Angular guide to writing [HTTP Interceptors is here](https://docs.angularjs.org/api/ng/service/$http#interceptors)

### General Conventions
* Write interceptors so they only chanage a request on certain conditions, so other unit tests don't have to be modified for the interceptors conditions
* Write interceptors so they only change a request on certain conditions, so other unit tests don't have to be modified for the interceptors conditions
* Don't include HTTP Interceptors in openlmis-core, as the interceptor might be injected into all other unit tests — which could break everything

### Unit Testing Conventions
Expand Down Expand Up @@ -223,11 +189,202 @@ Always lowercase camelCase the name of the object. Append 'Service' to the end o
```
beforeEach(module($provide){
// mock out a tape recorder service, which is used else where
tape = jasmine.createSpyObj('tape', ['play', 'pause', 'stop', 'rewind']);
this.tape = jasmine.createSpyObj('tape', ['play', 'pause', 'stop', 'rewind']);
// overwrite an existing service
var tape = this.tape;
$provide.service('TapeRecorderService', function(){
return tape;
});
});
```

## Class

Class is a our custom pattern that was designed to make our codebase easier to migrate to ES6 once we add support for it as it mimics how the ES6 classes looks like.

When to use:
* factories,
* services,
* domain classes that define some logic.

When not to use:
* interceptors,
* domain objects that does not have any logic.

### How to define
```Javascript
(function() {

'use strict';

/**
* @ngdoc service
* @name module-name.ClassName
*
* @description
* Example class.
*/
angular
.module('module-name')
.factory('ClassName', ClassName);

function ClassName() {

ClassName.prototype.exampleMethod = exampleMethod;

return ClassName;

/**
* @ngdoc method
* @methodOf module-name.ClassName
* @name ClassName
*
* @description
* Creates a new instance of the ClassName class.
*
* @param {Object} parameter the constructor parameter
* @return {Object} the object of the ClassName class
*/
function ClassName(parameter) {
this.parameter = parameter;
}

/**
* @ngdoc method
* @methodOf module-name.ClassName
* @name exampleMethod
*
* @description
* Creates a new instance of the ClassName class.
*
* @param {Object} parameter the method parameter
* @return {Object} the result of the method
*/
function exampleMethod() {
//do something
}
}

})();
```

### Extending classes

In order to extend a class we can use the classExtender factory that deals will basic prototype extension.

```Javascript
(function() {

'use strict';

/**
* @ngdoc service
* @name module-name.ClassName
*
* @description
* Example extending class class.
*/
angular
.module('shipment')
.factory('ExtendingClass', ExtendingClass);

ExtendingClass.$inject = ['ExtendedClass', 'classExtender'];

function ExtendingClass(ExtendedClass, classExtender) {

classExtender.extend(ExtendingClass, ExtendedClass);

return ExtendingClass;

function ExtendingClass() {
//add the following line to call the constructor of the super class
this.super.apply(this, arguments);
}
}

})();
```

### Singletons

In order to create a singleton we should still elevate AngularJS dependency injection capabilities. The className object will be shared across the whole application and can be injected using AngularJS dependency injection.

```Javascript
(function() {

'use strict';

/**
* @ngdoc service
* @name module-name.ClassName
*
* @description
* Example class.
*/
angular
.module('module-name')
.factory('ClassName', ClassName);

function ClassName() {

ClassName.prototype.exampleMethod = exampleMethod;

return ClassName;

/**
* @ngdoc method
* @methodOf module-name.ClassName
* @name ClassName
*
* @description
* Creates a new instance of the ClassName class.
*
* @param {Object} parameter the constructor parameter
* @return {Object} the object of the ClassName class
*/
function ClassName(parameter) {
this.parameter = parameter;
}

/**
* @ngdoc method
* @methodOf module-name.ClassName
* @name exampleMethod
*
* @description
* Creates a new instance of the ClassName class.
*
* @param {Object} parameter the method parameter
* @return {Object} the result of the method
*/
function exampleMethod() {
//do something
}
}

})();

(function() {

'use strict';

/**
* @ngdoc service
* @name module-name.ClassName
*
* @description
* Example class.
*/
angular
.module('module-name')
.factory('className', className);

className.$inject = ['ClassName'];

function className(ClassName) {
return new ClassName();
}

})();
```
Loading

0 comments on commit ca45fea

Please sign in to comment.