10
\$\begingroup\$

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:

Folder structure

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

<p>{{time | date:'HH:mm' }}</p>

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<Date>;

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

  getClock(): Observable<Date> {
    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?

\$\endgroup\$
7
\$\begingroup\$

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.

\$\endgroup\$
  • 2
    \$\begingroup\$ If you use {{time | async | date:'HH:mm' }} it will automagically unsubscribe. \$\endgroup\$ – John Neuhaus Jun 21 at 3:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.