11
\$\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?

asked Feb 21, 2017 at 16:21
\$\endgroup\$

1 Answer 1

9
\$\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.

answered Apr 22, 2017 at 7:22
\$\endgroup\$
1
  • 4
    \$\begingroup\$ If you use {{time | async | date:'HH:mm' }} it will automagically unsubscribe. \$\endgroup\$ Commented Jun 21, 2019 at 3:42

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.