HiveBrain v1.2.0
Get Started
← Back to all entries
patterntypescriptMinor

Angular 2 clock with RxJS Observable

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withangularobservableclockrxjs

Problem

I'm fairly new to Angular 2 and I started off with creating a clock for my app. I tried to stick to the official documentation tutorial.

Folder structure:

The CSS file is still empty and the HTML is not very spectacular:

{{time | date:'HH:mm' }}


The clock component itself looks like this:

import {Component} from "@angular/core";
import {ClockService} from "./clock.service";

@Component({
  selector: 'clock',
  templateUrl: './clock.component.html',
  styleUrls: ['./clock.component.css']
})
export class Clock {

  time: Date;

  constructor(private clockService: ClockService) {
  }

  ngOnInit() {
    this.clockService.getClock().subscribe(time => this.time = time);
  }

}


The most important part, however, is the service, which looks like this:

import {Injectable} from "@angular/core";
import {Observable} from "rxjs";

@Injectable()
export class ClockService {

  private clock: Observable;

  constructor() {
    this.clock = Observable.interval(1000).map(tick => new Date()).share();
  }

  getClock(): Observable {
    return this.clock;
  }

}


Basically, I need a general review since this is my very first approach. However, what I'm really curious about is if I get the usage of the Observable right. Is it correct to store it as a private field like shown above? Am I doing the .share() correctly?

Solution

Your service looks perfectly fine to me. As far as the Clock component goes, I can only recommend one thing.

When getClock().subscribe(...) is executed, a reference to the subscription is being returned back. Ideally, the reference should be kept as long as the component is alive. On component destruction, the subscription should be released by invoking subscription.unsubscribe().

@Component({
  selector: 'clock',
  templateUrl: './clock.component.html',
  styleUrls: ['./clock.component.css']
})
export class Clock implements OnInit, OnDestroy {

  private _clockSubscription: Subscription;
  time: Date;

  constructor(private clockService: ClockService) { }

  ngOnInit(): void {
    this._clockSubscription = this.clockService.getClock().subscribe(time => this.time = time);
  }

  ngOnDestroy(): void {
    this._clockSubscription.unsubscribe();
  }

}


P.S. As a minor side note, I want to mention that some devs like to explicitly mention the OnInit/OnDestroy/... interfaces in the class' implements clause. I am not sure whether it's very helpful or a big deal, but at least the compiler will complain if the ngOnInit is missing while class declares that OnInit is implemented.

Code Snippets

@Component({
  selector: 'clock',
  templateUrl: './clock.component.html',
  styleUrls: ['./clock.component.css']
})
export class Clock implements OnInit, OnDestroy {

  private _clockSubscription: Subscription;
  time: Date;

  constructor(private clockService: ClockService) { }

  ngOnInit(): void {
    this._clockSubscription = this.clockService.getClock().subscribe(time => this.time = time);
  }

  ngOnDestroy(): void {
    this._clockSubscription.unsubscribe();
  }

}

Context

StackExchange Code Review Q#155944, answer score: 9

Revisions (0)

No revisions yet.