btroncone/ngrx-examples

Technical Review?

NathanWalker opened this issue · 4 comments

btroncone commented

No problem, I'll take a look and let you know a bit later today. Thanks for your interest!

btroncone commented

A couple things I noticed:

1.) You generally will not need to call combine reducers, provideStore will combine reducers behind the scenes when you pass an object map of reducers.
2.) For storing route state, you can check out this middleware.
3.) It looks like you may be maintaining seperate state in one of your services (based on this call this.multilang.changeLang(action.payload.lang)). It would be better to move all state up to store and dispatch an action to update language state.
4.) Since your action service is really dispatching one action, you could dispatch the action from your service method rather then wrapping in a Subject. You could even remove the service and inject dispatcher into component for this use-case.
5.) Since 1.3 there is no need to dispatch from subscribe like this: .subscribe((action: Action) => store.dispatch(action));. Instead, since store is a BehaviorSubject, you can simply subscribe store to your actions$ stream: .subscribe(store);

Just a few thoughts after a cursory glance, I think it looks great! Good luck with your project!

NathanWalker commented

Excellent feedback @btroncone Thank you so much for your time looking at it!
This really helps my understanding, I'll make adjustments based on this soon, you're the best!

btroncone commented

I thought you responded with another question but I do not see it here now. Feel free to message me on gitter at any time if you have additional questions, I don't mind at all!