37
loading...
This website collects cookies to deliver better user experience
Unit Testing: If you don‘t have any test create them upfront. Each step is a small refactoring: you may break something. For the guys of you who sit in front of really untestable code: I understand your situation. All ”dangerous” steps are marked. You need to test much more and manually. At least introduce unit tests afterwards. Dependency injection is your friend.
do
operators. Whenever you read a do
, you know: that‘s a side effect. If you want a slightly more expressive variant try @molecule/do-next
, @molecule/do-error
and @molecule/do-complete
.// previous code
myObservable$.subscribe(
next => handleNext(next),
error => handleError(error),
() => handleCompletion(),
);
// refactored code
myObservable$.do(
next => handleNext(next),
error => handleError(error),
() => handleCompletion(),
)
.subscribe();
/* or with @molecule/do-next,
* @molecule/do-error,
* @molecule/do-complete
*/
myObservable$.do(
next => handleNext(next),
error => handleError(error),
() => handleCompletion(),
)
.subscribe();
() => {}
). Especially if we extract all side effects from our operators, we know: all side effects live within do*
and finally
! Every statement that is not your return
-statement and no variable declaration, that is used for your return
is a side effect. If the operator is a catch
, move the side effects logically isolated (see above) to do(undefined, yourErrorHandler)
/doError(yourErrorHandler)
. Otherwise extract them to do
/doNext
.// previous code
myObservable$.catch(error => {
console.log(’Could not resolve myObservable$’, error);
return Observable.empty();
})
// refactored code
myObservable$
// or use .do(undefined, errorHandler)
.doError(error => console.log(
’Could not resolve myObservable$’,
error
))
.catch(() => Observable.empty());
private
method of your class. If it doesn’t even depend on this
you can actually make it static
. The following case may seem a bit awkward at first, but it will keep everything simple and stupid. You will see, one can totally understand previewOfFavorites$
without knowing all those details. And if the underlying API changes, our public methods with all of our business logic just don‘t care.// previous code
public get previewOfFavorites(): Observable<Favorite[]> {
const previewSize$ = this.user.settings$.map(settings => settings.preview.size);
const favorites$ = this.user.favorites$();
return Observable.combineLatest(
previewSize$,
previewOfFavorites$,
(size, favorites) => favorites.slice(0, 3)
);
}
// refactored code
public get previewOfFavorites$(): Observable<Favorite[]> {
return Observable.combineLatest(
this.previewSize$,
this.previewOfFavorites$,
(size, favorites) => favorites.slice(0, 3)
);
}
private get previewSize$(): Observable<number> {
return this.user.settings$.map(settings => settings.preview.size);
}
private get favorites$(): Observable<Favorite[]> {
return this.user.favorites$();
}
return
-statements as handlers and transformations. In that case we just use the short notation for closures. One exception are object literals, which need to be wrapped in parentheses (() => ({})
) or leave the explicit return statement if you prefer it.// previous code
myObservable$.map(value => {
return [value];
})
// refactored code
myObservable$.map(value => [value]);
Replace instance variables through Observables they represent
Remove temporary observable and adjust mergeMap
s
If extracted subject with object for parameter, adjust function parameters
Trigger actions with temporary observable named *Action
(Observable.empty().finally
)
Sideeffect at the begginning before current operator, otherwise behind
Keep functions which prevent execution
All subjects should always be private: add accessors processDidChange
, processDidFail
, processDidComplete
,processObserver
(just if required)