The Public init Method
This is usually, but not always, a result of sequencing issues regarding a subscription.
Example: if a service is subscribing to a ReplaySubject, but needs to actually get data that may not be ready yet, the subscribe call is often put into an init method to ensure .subscribe() is called after the first .next() is called.
While this solves the immediate sequencing issue, it does nothing to remedy why the sequencing was an issue to begin with. Why is there a possibility that it subscribes to the ReplaySubject before the first .next() is called? Why does that break things? Why is the observable a ReplaySubject at all?
Another cause of this smell is a sibling or inter-level dependency.
Example: service A notifies service B and service C, but service C needs a resulting object of service B’s response to service A. Therefore service C’s subscribe call is put in an init method and Service A’s Observable is made to be a ReplaySubject. ¯\_(ツ)_/¯
Property Checking
99% of the time this is to account for the scenario where a method is called when it should not be called.
Example: .unsubscribe() is called on a property but there is no subscription on the property.
Adding a property check will stop any stack traces, but it does not solve the underlying design problem. Why is it possible that .unsubscribe() could be called before anything has been subscribed to? Is that a symptom of an underlying structural problem?
Flags
While Angular has lured us into believing that flags are ok with their NgIf directive , there are no other cases where this will not end badly (and that one might too).
Flags do not scale. If you add a second flag, you now have not two, but four potential states to influence branching logic. O(n²) for cyclomatic complexity.
Flags need to be toggled at the right time. Time being the operative word there. Flags introduce another level of sequencing to control flow, and while it might be trivial in the beginning, it can spin out of control quickly.
The Early Return
This one might get some push back. Yes it’s handy. No it doesn’t cause any direct harm. But why was the function called in the first place if all it was going to do was nothing?
This usually indicates a deficiency in, or the absence of, upstream branching logic.
Example: Subject A has subscribers X,Y,Z. A’s payload in the .next() call is boolean. X,Y, and Z’s response methods return early if the boolean is true, false, true respectively.
Let’s say Subject A is broadcasting on .interval of 200ms and every minute the payload is false. While that results in a ton of unnecessary calls, it also creates a pattern where it’s OK to subscribe to something when you only need to be notified a fraction of the time.
The issue isn’t with the early return itself, the issue is what it can hide. Much like property checking, it will hide stack traces but it will not resolve or illuminate any structural problems.
Oldest comments (0)