Observer #19
Observer #19
Conversation
|
|
||
| ## Problem statement | ||
|
|
||
| - We would like to keep the bag badge count up to date. | ||
| - The observer pattern allows us to notify the bag icon view with the updated badge count when a product is added or removed. | ||
| - Each Apple Store customer's bag contains an array of products, which are stored on the server. |
| - The observer pattern allows us to notify the bag icon view with the updated badge count when a product is added or removed. | ||
| - Each Apple Store customer's bag contains an array of products, which are stored on the server. | ||
|
|
||
| - When on the Apple Store bag list view, we would like to keep the array of products up to date, even if they are added or removed from another device. |
There was a problem hiding this comment.
Cart view. List is redundant in this context.
|
|
||
| - When on the Apple Store bag list view, we would like to keep the array of products up to date, even if they are added or removed from another device. | ||
|
|
||
| - In the top navigation, we would also like the bag icon view badge count to be in sync with the bag product count. |
There was a problem hiding this comment.
Search for all instances of "bag"
There was a problem hiding this comment.
"cart icon badge count"
"view" is redundant.
|
|
||
| - Both the bag icon view and bag list view need to be updated when products are added or removed. | ||
|
|
||
| - We would like to avoid having to check for changes in each individual view. |
There was a problem hiding this comment.
Can you make this problem statement stronger? How about: "Without implementing the observer patter views would need to fetch data in order to stay up to date, and this may result in unnecessary fetching, or the views not staying in sync with the model.
|
|
||
| ## Domain application | ||
| - This allows for a decoupled design where there is a clear separation of concerns between the subject and the observers, conforming to the single responsibility principle. |
There was a problem hiding this comment.
I'm not sure it's true that NOT conforming to the observer pattern would mean we are violating SRP. Furthermore, using the Observer pattern doesn't guarantee it either. Suggest removing ", conforming to the single responsibility principle"
| func attach(observer: BagObserver) | ||
| func detach(observer: BagObserver) | ||
| func notify() | ||
| protocol Observer: AnyObject { |
There was a problem hiding this comment.
Again, why the need for AnyObject?
There was a problem hiding this comment.
Was needed for the observer removal method, but since it's not used in the example, leaving a comment instead
|
|
||
| ```swift | ||
| protocol BagObserver { | ||
| func updateBagCount(_ count: Int) | ||
| procotol ObserverManaging { |
There was a problem hiding this comment.
Spelling: protocol not procotol
|
|
||
| func attach(observer: BagObserver) { | ||
| observers.append(observer) | ||
| func testNotificationAfterAddingProduct(_ product: Product) { |
There was a problem hiding this comment.
Why is this method called "test"
There was a problem hiding this comment.
Oh, I see, you added it so your example works. I get it, but I'm not a fan of adding code like this to examples. It's confusing to the reader.
| func addProduct(_ product: Product) { | ||
| self.products.append(product) | ||
| } |
There was a problem hiding this comment.
When a product is added surely you want to notify observers?? There should be a call to notify in this method, and notify should be private.
| notifier.addProduct( | ||
| Product(name: "iPad Pro", price: 999.99) | ||
| ) | ||
| notifier.notify() |
There was a problem hiding this comment.
Shouldn't need to call notify on the notifier. The class should be set up to handle notifications when its data changes.
No description provided.