Clean Code Compendium

The Clean Code Compendium is an opinionated guide for keeping code clean, readable, and maintainable. It covers a mix of both best practices and code style.

Code style takes inspiration from the Angular Style Guide. Code styles that are not documented here should consider using the Angular Style Guide as a fallback.

Sections:

Angular
Redux (NgRx)
RxJS
General

Angular

OnPush Change Detection

Avoid leaving changeDetection undefined in @Component
Do define ChangeDetectionStrategy.OnPush explicitly
Why? Improve performance, minimise CD cycles

/* Avoid */
@Component({
  selector: x,
  templateUrl: y,
})

/* Do this instead */
@Component({
  selector: x,
  templateUrl: y,
  changeDetection: ChangeDetectionStrategy.OnPush,
})

Single Responsibility Principle

Do try to keep components focused only on the display-logic e.g. showing/hiding, etc
Do try to extract business logic out into services
Why? Keeps classes focused only on their 1 job

/* Avoid */
public class MyComponent {
  public isInternalDirectDebit() { }
}

<div
  *ngIf="isInternalDirectDebit()">
</div>

/* Do this instead */
public class MyComponent {
  public showSection;

  ngOnInit() {
    this.showSection = this.getShowSection();
  }

  public getShowSection() {
    return this.service.isInternalDirectDebit();
  }
}

<div
  *ngIf="showSection">
</div>

Unused @Inputs/@Outputs
- Avoid creating unused @Inputs/@Outputs "just in case they may be useful"
- Why? You Ain't Gonna Need It (YAGNI)
Container vs Presentational components
- Do allow child/nested components to be container/smart components (e.g. inject services)
- Avoid forcing @Inputs to be passed down a whole chain of parent/child components
- Why? Prevents duplicate @Inputs on multiple parent/child components
- Why? Prevents bloating up the 1 top-level container/smart component with vars just purely to pass them down
Method calls in templates
- Avoid invoking complex methods in template
- Do memoize the method if applicable
- Do use a var to hold the value
- Do initialise the var in OnInit and/or update in OnChanges
- Why? Methods in template can be invoked as often as every CD cycle
- Why? Prevents invoking the method repeatedly and executing the complex logic repeatedly
```
/* Avoid */
<div>{{ getMeaningOfLife() }}</div>

/* Do this instead */
public meaningOfLife

ngOnChanges() {
  this.meaningOfLife = this.getMeaningOfLife();
}

<div>{{ meaningOfLife }}</div>
```
Observables in templates
- Avoid using observables with the async pipe in templates
- Avoid calling methods returning observables in templates
- Do extend AbstractConnectableComponent
  - Do create a var to hold the latest value of the observable
  - Do initialise it in ngOnInit using this.connect()
- Why? Prevents repeated async pipe subscriptions to the same observable
- Why? Prevents async pipe clutter in the template
- Why? Prevents every CD cycle invoking the method and returning a new observable, and doing a new subscription, etc
```
/* Avoid */
<hello
  [world]="getWorld$()" | async>
</hello>

<foo
  [world]="world$" | async>
</foo>

/* Do this instead */
class MyComponent extends AbstractConnectableComponent {
  public world;

  ngOnInit() {
    this.connect({ world: this.getWorld$() });
  }
}

<hello
  [world]="world">
</hello>

<foo
  [world]="world">
</foo>
```

Complex expressions in templates

Avoid complex expressions in template
Do use a var to hold the value
Do initialise the var in OnInit and/or update in OnChanges
Why? Improves readability

/* Avoid */
<div
  *ngIf="a.b.c === 'hello' || d.e.f === 'world' && g.h.k !== 'foobar'">
</div>

/* Do this instead */
public showSection;

ngOnChanges() {
  const isHello = a.b.c === 'hello';
  const isWorld = d.e.f === 'world';
  const isNotFoobar = g.h.k !== 'foobar';

  this.showSection = isHello || isWorld && isNotFoobar;
}

<div
  *ngIf="showSection">
</div>

Aliases in templates

Do alias vars in templates to reference lengthy expressions
Why? Prevents repetition and improves readability

/* Avoid */
<div *ngIf="x | async">
  {{ x | async }}
</div>

/* Do this instead */
<div *ngIf="x | async as x">
  {{ x }}
</div>

Placement of <ng-template> in templates
- Do put the "real" html at the start of the template file
- Do put <ng-template>s at the bottom of the template file
- Why? Easier to understand what the component shows when glancing at start of file
```
/* Avoid */
<ng-template></ng-template>

<my-component></my-component>

/* Do this instead */
<my-component></my-component>

<ng-template></ng-template>
```
Naming event handler methods
- Do name events without the "on" prefix
- Do name event handler methods with the "on" prefix
- Do name event handlers methods after their event
- Why? Makes it obvious which methods handle events
```
/* Avoid */
(onClick)="doThis()"

/* Do this instead */
(click)="onClick()"
(click)="onClickAdd()"
```

Formatting component properties

Do group them by
- public static
- Inputs
- Inputs (Setter+Getter pairs)
- Inputs (multiple decorators)
- Outputs
- Other decorators
- public readonly
- public
- protected
- private
Do alphabetically order (case-sensitive) them within each group
Why? Improves readability, prevents duplicates

// Public static vars
public static HELLO: any;

// Inputs: Standard ones first
@Input public a: string;    // in-lined decorator with var declaration
@Input public b: string;    // no empty line between declarations

// Inputs: Setters + Getters
@Input                     // decorator on its own line
public set c(c: string) {
  this._c = c;
}                          // no empty line between Setter + Getter pair
public get c(): string {
  return this._c;
}                          // empty line around the pair, just like other methods

@Input
public set d(d: string) {
  this._d = d;
}
public get d(): string {
  return this._d;
}

// Inputs: Multiple decorators
@Input
@ViewChild({ read: SomeDirective })   // each decorator on its own line
public e: string;
                                      // empty line between declarations
@Input
@ContentChild(SomeOtherDirective)
public f: string;

// Outputs
@Output public g: EventEmitter<any> = new EventEmitter();
@Output public h: EventEmitter<any> = new EventEmitter();

// Other decorators in their own groups
@ContentChild(MyDirective) public i: any;
@ContentChild(YourDirective) public j: any;

@ViewChild('HisTemplate') public k: any;
@ViewChild('HerTemplate') public l: any;

// Public readonly vars
public readonly WORLD: any;

// Public vars
public m: string;
public n: string;

// Protected static vars
protected static foo: string;

// Protected readonly vars
protected readonly bar: string;

// Protected vars
protected o: string;
protected p: string;

// Private static vars
private static cow: string;

// Private readonly vars
private readonly moo: string;

// Private vars
private _c: string;   // underscored vars first
private _d: string;
private q: string;
private r: string;

Formatting modules

Do separate 3rd party modules from internal modules
Do alphabetically order (case-sensitive) modules in each group
Do alias Angular modules by prefixing with Ng
Why? Prevents duplicates

/* Avoid */
imports: [
  ZModule3rdParty,
  AModule,
  NgCommonModule,
  YModule3rdParty,
  BModule,
]

/* Do this instead */
imports: [
  NgCommonModule,
  YModule3rdParty,
  ZModule3rdParty,

  AModule,
  BModule,
],
declarations: [
  AComponent,
  BComponent,
]

Formatting properties on elements
- Do group them into 3 groups:
  - Directives/Attributes
  - Inputs
  - Ouputs
- Do alphabetically order (case-sensitive) them within each group
- Do keep the 1st attribute on the same line as the element
- Do use newlines for other additional attributes
- Why? Improves readability, prevents duplicates
```
/* Avoid */
<hello id="" class="" *ngIf="" [abc]="" [def]="" (abc)="" (def)=""></hello>

/* Do this instead */
<hello *ngIf=""
  abcDirective
  class=""
  defDirective
  id=""
  [abc]=""
  [def]=""
  (abc)=""
  (def)="">
</hello>
```
Access modifiers
- Do keep access modifiers to the smallest scope possible
  - private by default
  - protected if subclasses need access
  - public if external classes or template needs access
- Why? Better encapsulation
Providers
- Avoid adding a class to the providers array if it is not being injected as a dependency
- Why? Unnecessary
- Why? Only classes that need to be injected as dependencies should be in providers
```
/* Avoid */
providers: [
  InjectableOne,
  InjectableTwo,
  NonInjectableClass
]

/* Do this instead */
providers: [
  InjectableOne,
  InjectableTwo
]
```
Inputs as observables
- Avoid having complex @Input setters
- Avoid having complex logic inside ngOnChanges to react to input value changes
- Do create a private var to hold an observable version of the @Input
- Do use ngOnChanges to push new values into the var
- Why? Current value of @Input is accessible as normal
- Why? Changes to the value of @Input can be handled via observables
- Why? Simplifies code in ngOnChanges
```
/* Avoid */
@Input public set x(x) { ... }

ngOnChanges(changes) {
  if (changes.x) { ... }
}

/* Do this instead */
@Input public x;

private x$: Subject<any>;

ngOnChanges({ x: xChange }: SimpleChanges) {
  if (xChange) {
    this.x$.next(xChange.currentValue);
  }
}
```

Redux (NgRx)

Folder structure

Do create separate files for state, actions, selectors, reducer, effects
Why? Separation of concerns, easier to understand

/* Do this */
x.state.ts     // state interface, initial state obj, entity adapter
x.actions.ts   // action classes used by reducer, effects
x.selectors.ts // selectors and pipeable operators for reading a slice of state
x.reducer.ts   // pure functions for updating the state from a given action
x.effects.ts   // side effects

Reducers vs Effects
- Do use reducer actions to handle logic where possible
- Do use effects when dependency injection (services) or async calls are needed
- Why? Reducer actions are pure functions, easier to test, etc

Redux action strings

Do define the action string as [path to value][action][status]
Why? Easier to identify what is being updated

/* Avoid */
const ADD_VALUATION_SUCCESS = '[valuation][add][valuerValuation][request][success]'

/* Do this instead */
const ADD_VALUATION_SUCCESS = '[valuation][valuerValuation][request][add][success]'

Service-wrappers for Store
- Avoid creating services/methods to wrap the store
- Do inject the store and use the selector, action directly
- Do create selectors to access the exact slice of state needed
- Do move logic from service methods into effects if appropriate
- Why? Makes actions more self-contained, without relying on service methods performing logic beforehand
- Why? Prevents problems with combineLatest(service.x$, service.y$) firing multiple times when the state updates a single time
- Why? Prevents services from bloating up and becoming a dumping ground for methods
- Why? More pure functions (selectors, utils)
```
/* Avoid */
service.getX$();
service.patchX(x);

/* Do this instead */
this.store.select(XSelectors.x);
this.store.dispatch(new PatchXAction(x));
```

RxJS

Avoid over-using RxUtils.sync()
- Avoid over-reliance on RxUtils.sync()
- Do compose observables instead of using RxUtils.sync()
- Do only use it when no longer working with async
- Why? Prevents unnecessary subscriptions
```
/* Avoid */
const x = RxUtils.sync(x$);
return y$.pipe(map(y => y && x));

/* Do this instead */
return y$.pipe(
  withLatestFrom(x$),
  map(([y, x]) => y && x),
)
```

Compose RxJS

Avoid complex logic in map or mergeMap
Do break up observables beforehand, and compose them later
Why? Improves readability

/* Avoid */
x$.pipe(
  pairwise(),
  map(([previousX, currentX]) => {
    const previousXProp = previousX.prop;
    const currentXProp = currentX.prop;
    return previousXProp !== currentXProp;
  })
)

/* Do this instead */
const xProp$ = x$.pipe(map(x => x.prop));

xProp$.pipe(
  pairwise(),
  map(([previousXProp, currentXProp]) => previousXProp !== currentXProp),
)

Long-running mergeMap

Do be careful when using mergeMap, as the inner observable will keep running
Do use take(1) or change to withLatestFrom() if the inner observable only needs to run once
Why? Prevents bugs from observables triggering even after initial event is over

// Beware! Even if x$ only emits once, this will continue emitting when y$ emits
x$.pipe(
  mergeMap(() => y$)
)

// This only takes 1 emit from y$
x$.pipe(
  mergeMap(() => y$),
  take(1),
)

or

// This only takes the current value in y$
x$.pipe(
  withLatestFrom(y$),
)

Unsubscribing when needed

Do remember to unsubscribe when done (e.g. ngOnDestroy)
Why? Prevents subscriptions from continuously running

/* Avoid */
ngOnInit() {
  x$.subscribe(...)
}

/* Do this instead */
private readonly destroy$: Subject<void> = new Subject();

ngOnInit() {
  x$
    .pipe(takeUntil(this.destroy$))
    .subscribe(...)
}

ngOnDestroy() {
  this.destroy$.next();
}

Unsubscribing when unneeded

Avoid unsubscribing when unneeded
- e.g. take(1)
- e.g. http (observables that complete after 1 emit)
Why? Prevents redundant code

/* Avoid */
this.xSubscription = x$.pipe(
  take(1)
).subscribe(...)

this.xSubscription.unsubscribe();

/* Do this instead */
this.xSubscription = x$.pipe(
  take(1)
).subscribe(...)

/* Avoid */
this.httpSubscription = this.http.get().subscribe();

this.httpSubscription.unsubscribe();

/* Do this instead */
this.http.get().subscribe();

General

Passing arguments

Do pass only what is needed
Avoid passing unnecessary data where practical
Why? Easier to read and understand
Why? Prevents unnecessary referencing

/* Avoid */
<div (click)="onClick(application)"></div>

public onClick(application: Application) { }

/* Do this instead */
<div (click)="onClick(application.id)"></div>

public onClick(applicationId: Id) { }

Nested Ifs

Avoid nesting if blocks
Do move "exit" conditions to the start
Why? Improves readability

/* Avoid */
if (x) {
  if (y) {
    this.doWork();
  }
}

/* Do this instead */
if (!x || !y) {
  return;
}

this.doWork();

Comments
- Avoid adding code comments
- Do come up with meaningful var/method names so that they are self-documenting
- Do make an exception for code that may do something unexpected, e.g. code for performance optimisation
- Why? Comments usually become outdated, causing confusion later
```
/* Avoid */
// adds one
public doWork(x) {
  return x + 1;
}

/* Do this instead */
public addOne(x) {
  return x + 1;
}
```

Mutating args

Avoid mutating input arguments/parameters
Why? Prevents consumer of method from having to deal with unexpected behaviour

/* Avoid */
public hello(world) {
  world.foobar = 'foobar';
}

/* Do this instead */
public hello(world) {
  return {
    ...world,
    foobar: 'foobar',
  }
}

Prefixing with underscore

Do prefix with underscore to prevent name conflicts or to indicate the var is intended to be of local scope
Why Improves consistency of var names

/* Avoid */
public findBunny(bunny) {
  return bunnies.find(rabbit => rabbit === bunny);
}

/* Do this instead */
public findBunny(bunny) {
  return bunnies.find(_bunny => _bunny === bunny);
}

Consistent names

Do keep var names consistent with their type/collection
Why Improves clarity

/* Avoid */
public hello: World;

/* Do this instead */
public world: World;

/* Avoid */
rabbits.map(x => ...)

/* Do this instead */
rabbits.map(rabbit => ...)

Simple methods and functions

Do keep methods small in scope/purpose
Avoid bloating a method up with too much logic
Why? Improves readability and reuseability

/* Avoid */
public blah() {
  if (x < 5) {
    ...
    ...
    ...
  }

  if (x > 10) {
    ...
    ...
    ...
  }
}

/* Do this instead */
public blah() {
  if (x < 5) {
    this.doSomething();
  }

  if (x > 10) {
    this.doSomethingElse();
  }
}

Formatting chained method calls

Do use a single line for single, simple method calls
Do use a newline each for multiple method calls
Why? Improves readability

/* Avoid */
x.filter(...).map(...).find(...)

/* Do this instead */
x
  .filter(...)
  .map(...)
  .find(...)

/* Avoid */
x
  .map(...)

/* Do this instead */
x.map(...)

Standard methods over lodash
- Do prefer standard ES methods instead of lodash unless lodash offers functional benefits
- Why? Prevents inflating bundle size
```
/* Avoid */
find(things, (thing) => ...)

/* Do this instead */
things.find(thing => ...)
```
Dead/Unused code
- Avoid creating unused code "just in case it may be useful"
- Why? You Ain't Gonna Need It (YAGNI)
- Why? Prevents unnecessary complications, extra unit tests, etc

Types

Do define the var type
Do define the return type
Avoid defining the type if TS can already infer it
Why? Because. this. is. SPARTAAAA!!!

(If you don't like using types in Typescript, go back to Javascript)

/* Avoid */
public blah(blah) { }

/* Do this instead */
public blah(blah: string): void { }

public getX(): X { }

/* Avoid */
const x: X = this.getX();

/* Do this instead */
const x = this.getX();

Unit test method names
- Do name method test suites with parenthesis
- Why? Easier to identify the method being tested
```
/* Avoid */
describe('methodName', ...);

/* Do this instead */
describe('methodName()', ...);
```

Unit test descriptions

Do use BDD style descriptions
Why? Easier to read and group related specs

/* Avoid */
it('should return x when y', ...);

/* Do this instead */
describe('when y', () => {
  it('should return x', ...);
});

Ordering of properties
- Do order properties alphabetically (case-sensitive) (per group)
- Why? Prevents conflicts and avoid duplicates
```
/* Avoid */
public c;
public a;
public b;

/* Do this instead */
public a;
public b;
public c;
```
Spaces in arrays/square brackets
- Avoid using a space before/after square brackets
- Why? Improves readability
```
/* Avoid */
[ 1, 2, 3 ]

/* Do this instead */
[1, 2, 3]
```
Spaces between curly braces
- Do use a separating space before/after curly braces
- Why? Improves readability
```
/* Avoid */
{a: 'hello'}

/* Do this instead */
{ a: 'hello' }
```

Trailing commas

Do use trailing commas for objects, arrays
Why? Prevents merge conflicts

/* Avoid */
['hello', 'world']

/* Do this instead */
[
  'hello',
  'world',
]

Structure by feature

Do group files by feature rather than by layer
Why? Promotes code cohesion

/* Avoid */
/reducers
  - abc.reducer
  - def.reducer
/effects
  - abc.effects
  - def.effects
/services

/* Do this instead */
/abc
  - abc.reducer
  - abc.effects
  - abc.service
/def

Formatting constructors

Do use a single-line for single-arg constructors
Do use newlines for multi-arg constructors
Do alphabetically order (case-sensitive) them
Why? Improves clarity, and prevents duplicates

/* Avoid */
constructor(
  a: A,
)

/* Do this instead */
constructor(a: A)

/* Avoid */
constructor(b: B, c: C, a: A)

/* Do this instead */
constructor(
  a: A,
  b: B,
  c: C,
)

Meaningful var and method names

Do use vars and/or methods to provide more context
Do ensure var/arg names are meaningful
Why? Adds more meaning or explanation for the var/method

/* Avoid */
if (index === ownerships.length -1)

/* Do this instead */
const lastIndex = ownerships.length - 1;
if (index === lastIndex)

/* Avoid */
public onIsValid(event: boolean) { }

/* Do this instead */
public onIsValid(isValid: boolean) { }

Naming of observable vars/methods

Do name vars/methods that hold/return observables with a $ suffix
Why? Makes it clear that we are working with async vars

/* Avoid */
public hello: Observable<any>;

/* Do this instead */
public hello$: Observable<any>;

/* Avoid */
public getWorld(): Observable<any>

/* Do this instead */
public getWorld$(): Observable<any>

Lengthy if statements

Do use consts to break up lengthy expressions in if statements
Why? Improves readability

/* Avoid */
if ((this.desiredRepaymentMethodHasChanged(previousProduct, currentProduct) || this.selectedProductHasChanged(previousProduct, currentProduct))
  && (currentProduct.productDefinitionId && this.productsService.isDirectLoanRepayment(currentProduct)))

/* Do this instead */
const hasSelectedProductChanged = this.hasSelectedProductChanged(previousProduct, currentProduct);
const hasDesiredRepaymentMethodChanged = this.hasDesiredRepaymentMethodChanged(previousProduct, currentProduct);
const hasProductDefinition = !!currentProduct.productDefinitionId;
const isDirectLoanRepayment = this.productsService.isDirectLoanRepayment(currentProduct);

if ((hasDesiredRepaymentMethodChanged || hasSelectedProductChanged) && (hasProductDefinition && isDirectLoanRepayment))

Variable declarations
- Do prefer to use const
- Avoid using let
- Never use var
- Why? Improves safety and prevents unintended variable reassignment
```
/* Avoid */
var x = 5;
let y = true;

/* Do this instead */
const x = 5;
const y = true;
```
Fat arrow function args
- Avoid using parenthesis for single, untyped arg
- Do use parenthesis for single, typed arg (only if type cannot be inferred)
- Do use parenthesis for multiple args
- Avoid using return braces for simple/single-line statements
- Do use return braces for complex/multi-line statements
- Why? Improves readability
```
/* Avoid */
(x) => ...

/* Do this instead */
x => ...
(x: string) => ...
(x, y) => ...
```
```
/* Avoid */
x => {
  return doSomething(x);
}

/* Do this instead */
x => doSomething(x)
x => {
  const y = ...
  const z = ...

  return ...
}
```
Bracket pairs
- Do keep both opening and closing brackets on same line for simple functions
- Do put opening and closing brackets on new lines for complex functions
- Why? Improves readability
```
/* Avoid */
do(x => {
  return 5; });

/* Do this instead */
do(x => 5);
do(x => {
  return 5;
});
```

Name		Name	Last commit message	Last commit date
e2e		e2e
src		src
.browserslistrc		.browserslistrc
.editorconfig		.editorconfig
.gitignore		.gitignore
README.md		README.md
angular.json		angular.json
karma.conf.js		karma.conf.js
package-lock.json		package-lock.json
package.json		package.json
tsconfig.app.json		tsconfig.app.json
tsconfig.base.json		tsconfig.base.json
tsconfig.json		tsconfig.json
tsconfig.spec.json		tsconfig.spec.json
tslint.json		tslint.json

Navigation Menu

Repository navigation

About

Repository files navigation

Clean Code Compendium

Angular

Redux (NgRx)

RxJS

General

Angular

Redux (NgRx)

RxJS

General

About

Releases

Packages

Used by

Contributors

Languages

Latest commit

History

Folders and files

Repository files navigation

Clean Code Compendium

Angular

Redux (NgRx)

RxJS

General

Angular

Redux (NgRx)

RxJS

General

About

Resources

Stars

Watchers

Forks

Releases

Packages

Used by

Contributors

Languages