-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Builder #27
base: main
Are you sure you want to change the base?
Conversation
|
||
- No matter which collection and options are chosen, the API where we submit configured Apple Watches remains the same. | ||
|
||
- To avoid the problem of steering away from this requirement, we can use the Builder pattern, which allows for flexibility in the construction process, while still maintaining a consistent output. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very weak problem statement. The problem is essentially: "let's avoid steering away from the solution".
|
||
Director: | ||
class AppleWatchHèrmesSeries10: AppleWatchBuilder { | ||
var appleWatch = AppleWatch(collection: "Hèrmes Series 10", size: Size.fortyTwo.rawValue, material: Material.titanium.rawValue, band: Band.torsade.rawValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"apple" is somewhat redundant in the name of this property. The class on which the property is defined already contains this info.
So:
AppleWatchHèrmesSeries10.appleWatch
vs
AppleWatchHèrmesSeries10.watch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with the different naming: HèrmesSeries10Buider.build()
And private: appleWatch
|
||
```swift | ||
class CheckoutOrderBuilder: OrderBuilder { | ||
private var order = Order() | ||
class AppleWatchSeries10: AppleWatchBuilder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this should be called AppleWatchSeries10Builder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since that is what it IS. AppleWatchSeries10 is what it returns.
class CheckoutOrderBuilder: OrderBuilder { | ||
private var order = Order() | ||
class AppleWatchSeries10: AppleWatchBuilder { | ||
var appleWatch = AppleWatch(collection: "Series 10", size: Size.fortyTwo.rawValue, material: Material.aluminum.rawValue, band: Band.sportBand.rawValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can improve on this. In some examples I see guys including a build
method in their builder. I think this is a useful inclusion as it communicates clearly that this is the terminal method to call in order to get a watch.
So in this case, appleWatch
or just watch
becomes private, and we add a build method to return the watch configured by any calls to the other methods.
It's not a critical change, and as far as I know the build method is not included in Gof4, but build is used in other examples online and I personally think it's an improvement.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
associatedtype SizeType | ||
associatedtype MaterialType | ||
associatedtype BandType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice use of generics. I haven't tested this code. You 100% sure it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works
print(series10.appleWatch) // Default Series 10 Apple Watch | ||
// Output: AppleWatch(collection: "Series 10", size: "42mm", material: "Aluminum", band: "Sport Band") | ||
|
||
series10.setSize(.fortySix).setMaterial(.titanium).setBand(.milaneseLoop) // Update Series 10 Apple Watch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like how the builder pattern enables this chaining of methods. It's cool. Going to use this one in the future.
- The Builder pattern allows for the construction of a car with different features using the same construction process. | ||
- The Builder pattern is used to construct objects piece by piece. | ||
|
||
- Each part of the object is configured via builder methods. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Each part of the object is configured via a method on a builder object."
- Constructs and assembles parts of the product by implementing the Builder interface. | ||
- Defines and keeps track of the representation it creates. | ||
- Provides an interface for retrieving the product. | ||
- Compare the `Series10Builder` and `HèrmesSeries10Builder` builder enum types to see this in action. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Compare the Series10Builder
and HèrmesSeries10Builder
enum types to see this in action." - remove the word builder
No description provided.