3

I'm writing an application using Angular and find myself using this pattern constantly:

@Injectable(...)
export class WidgetRegsitryService {
  private readonly _widgets: BehaviorSubject<Widget[]> = new BehaviorSubject([]);
  public get widgets() { return this._widgets.value; }
  public readonly widgets$ = this._widgets.asObservable();

  public add(widget: Widget) {
    const old = this._widgets.value.slice();
    old.push(widget);
    this._widgets.next(old);
  }
}

A lot of services will have 3-5 or more such groups of public getters and private backing Subjects. It happens so much that the code feels very verbose and repetitive. So: a) is there a DRY way to do this, and b) am I misusing Observables here?

Coderer
  • 25,844
  • 28
  • 99
  • 154
  • I understand that (b) will be harder to answer without showing a concrete example, but I don't have anything reasonable to post. I know Ben Lesh has an answer somewhere around here saying that using `.value` means you're being "too procedural" or something, but it seems silly to me to make a service where you can't easily get the current value of a stateful property without `subscribe`ing to it. – Coderer May 24 '19 at 11:41
  • You can use a super class to reduce the amount of code, but it's definitely not a bad practice to use behavior subjects. The bad practice would be to use them when you don't need to. So ... Do you need to use them ? –  May 24 '19 at 11:45
  • I do. I sometimes start with a single property, then "promote" it to Observable when I need to act on changes (especially outside of a template). But this refactoring adds a lot of what I think of as boilerplate code, and it seems like there should be some kind of shorthand / convention that avoids being so dang wordy. – Coderer May 24 '19 at 11:48
  • the question is probably: who is subscribing to the subject above? If it's a component then you could also bind a public field of the service and listen on ngChanges inside the component – dannybucks May 24 '19 at 11:49
  • @JeremyBenks `OnChanges` is called only for `@Input`s –  May 24 '19 at 11:49
  • In case anybody comes across this in the future, [I found the Ben Lesh answer](https://stackoverflow.com/a/45227115/26286) I mentioned in a previous comment. – Coderer Sep 21 '21 at 10:50

2 Answers2

8

I'm writing an application using Angular and find myself using this pattern constantly:

The pattern you've shown is very similar to a state store such as; Redux, NgRX or NGXS. The difference is that you've placed the store, selectors and reducers into a single class.

Having everything in one place has advantages, but if you have to rewrite a new store every time you start a new service, then that would explain why you say "It happens so much that the code feels very verbose and repetitive".

There is nothing wrong with this, and there are many blog posts on the Internet that try to write a Redux clone in as few lines of code as possible. My point is that people are doing exactly what you're doing all the time.

private readonly _widgets: BehaviorSubject<Widget[]> = new BehaviorSubject([]);

The above is the store of the state manager. It's an observable that contains the current state and emits changes to that state.

public get widgets() { return this._widgets.value; }

The above is the snapshot of the state manager. This allows you to do specific calculates on the store without having to subscribe, but just as with other state stores using snapshots can have race condition problems. You should also never access this directly from a template, because it will trigger the "Expression has changed after it was checked" error.

public readonly widgets$ = this._widgets.asObservable();

The above is a selector for the store. Stores will often have many selectors that allow different parts of the application to listen for store changes on specific topics.

public add(widget: Widget) {
   const old = this._widgets.value.slice();
   old.push(widget);
   this._widgets.next(old);
   // above can be rewritten as
   this._widgets.next([...this.widgets, widget]);
}

We don't have the above in state store libraries. The above is broken down into two parts the action and the reducer. The action often contains the payload (in your case a widget) and the reducer performs the work of modifying the store.

When we use actions and reducers it decouples the business logic of how the store should change from the issues of reading the current state, updating the state and saving the next state. While your example is very simple. In a large application having to subscribe, modify and emit the changes can become overhead boilerplate code when all you want to do is toggle a boolean flag.

A lot of services will have 3-5 or more such groups of public getters and private backing Subjects. It happens so much that the code feels very verbose and repetitive.

You're entering the realm of reinventing the wheel.

As I see it, you have two possible options. Invent your own state store framework that will feel more comfortable for you, or use an existing state store from one of the libraries I listed above. We can't tell you which path to take, but I've worked on many Angular projects and I can tell you there is no right answer.

What really makes source code feel less verbose and repetitive is highly opinionated. The very thing that made it less verbose might one day come back to haunt you as a design mistake, and repetitive source code is pain but one day you'll be thankful you can modify a single line of code without it impacting other areas of your source code.

a) is there a DRY way to do this, and

The only way to dry out the source code is to decouple the implementation of state management from the business logic. This is where we get into a discussion of what makes a good design pattern for a state store.

  • Do you use selectors?
  • Do you use actions?
  • Do you use reducers?

Where do you want these things to be (in their own files, or methods of a service?). How do you want to name them, and should you re-use them or create new ones for every edge case?

It's a lot of questions that are really personal choices.

I can rewrite your example using NGXS as an example, but this might not look dry to you because frameworks need to be complex to be useful. What I can tell you is that it's easier to read documentation for NGXS when you need to do something you haven't done before, then trying to invent it yourself and risk getting it wrong. That doesn't mean NGXS is always right, but at least you can complain it's not your fault :)

@State<Widget[]>({
    name: 'widgets',
    defaults: []
})
export class WidgetState {
    @Action(AddWidgetAction)
    public add(ctx: StateContext<Widget[]>, {payload}: AddWidgetAction) {
        ctx.setState([...ctx.getState(), payload]);
    }
}

@Component({...})
export class WidgetsComponent {
    @Select(WidgetState)
    public widgets$: Observable<Widget[]>;

    public constructor(private _store: Store) {};

    public clickAddWidget() {
        this._store.dispatch(new AddWidgetAction(new Widget()));
    }
}

b) am I misusing Observables here?

Absolutely not misusing observables. You have a good grasp of why a service should be stateless and reactive. I think you're just discovering the value of state stores on your own, and now you're looking for ways to make the use of them easier.

Reactgular
  • 52,335
  • 19
  • 158
  • 208
  • I'm aware of Redux, and had the vague notion that I was accidentally reinventing it from first principles. Maybe I just need to read some more about it before our next major refactoring... – Coderer May 27 '19 at 12:20
0

A) To avoid a repetitive code for creating a BehaviorSubject, You can create a BehaviorSubject which contain a key and value and we subscribe by using a key so now no need to create a BehaviorSubject every time when we want to use.

Service

interface Event {
  key: string;
  value: any;
}


@Injectable({
  providedIn: 'root'
})

export class Broadcaster {

  // subject 
  protected _eventsSubject = new BehaviorSubject<Event>();
  constructor() {
  }

   broadcast(key: any, value: any) {
    this._eventsSubject.next({ key, value }); // here we are setting the key and value of our subject
   }

  on<T>(key: any): Observable<T> {
    return this._eventsSubject.asObservable()
            .pipe(
                filter(e => e.key === key),
                map(e => e.value)
            );
  }
}

componentOne

import { Broadcaster } from '../BrodcastService.service';
export class ComponentOne implements OnInit {
constructor(private broadcaster: Broadcaster) { }

someFunction() {

// here we are sending a data and setting a key of subject to 'msg1'

this.broadcaster.broadcast('msg1', 'data of msg1');

}

componentTwo

import { Broadcaster } from '../BrodcastService.service';
export class ComponentOne implements OnInit {
constructor(private broadcaster: Broadcaster) { }

someFunction() {

// here we are sending a data and setting a key of subject to 'msg2'

this.broadcaster.broadcast('msg2', 'data of msg2');
}

componentThree

import { Broadcaster } from '../BrodcastService.service';
export class ComponentOne implements OnInit {
constructor(private broadcaster: Broadcaster) { }

someFunction() {
// here we subscribe our subject and getting a value of msg1 key
    this.broadcaster.on('msg1').subscribe(resp => {
      console.log(resp);
    })
}
Yash Rami
  • 2,276
  • 1
  • 10
  • 16
  • I would still rather have 3 different subjects. – Roberto Zvjerković May 24 '19 at 14:14
  • Your solution doesn't strongly type the subjects -- if you don't use a generic parameter as the type of at least one input parameter, you're basically doing an anycast. You could solve this with a static type mapping from key to output type but at that point you're pretty close to the overhead of just making a bunch of subjects. – Coderer May 27 '19 at 12:19