Open
Conversation
(An attempt of) Implementing the Builder Design Pattern, generating csbs or nyt object using the BDP so that in the case where a new function is added in the future which doesn't require ALL the individual parts, the Builder can "cherry pick" the parts it needs and create a customized object for the new function to use, allowing greater flexibility. This could potentially reduce the number of parameters in the constructors to create the said customized csbs/nyt object, which would provide better readability, easier to read/understand what parameters the customized object has. Note that this (attempt of) implementation is meant to support the original functions, and that the implementation having LOTS of duplicated code (from csbs.py and nyt.py) suggests that the current implementation can be further refactored so that "code smell" can occur less. *regarding proper implementation* Location class and TimelinedLocation class in __init__.py should be removed as it is replaced by LocationBuilderInterface and TimelinedLocationBuilderInterface respectively as the Abstract interface. CSBSLocation class in csbs.py should be removed as it is replaced by CSBSLocationBuilder as the concrete Builder. similarly, NYTLocation with NYTLocationBuilder in nyt.py The Director and Product current located in csbs.py and nyt.py should be in their own respective module/file so that it is better organized and structured. (but it still works right now) The client code at the end of csbs.py and nyt.py should belong to another module/file where the other client codes are. The pseudo code of implementation have some lines of code commented out, which should reflect how it should looks like when properly implemented; Those lines of code are currently commented out to prevent program from unable to compile.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(An attempt of) Implementing the Builder Design Pattern, generating csbs or nyt object using the BDP so that in the case where a new function is added in the future which doesn't require ALL the individual parts, the Builder can "cherry pick" the parts it needs and create a customized object for the new function to use, allowing greater flexibility.
This could potentially reduce the number of parameters in the constructors to create the said customized csbs/nyt object, which would provide better readability, easier to read/understand what parameters the customized object has.
Note that this (attempt of) implementation is meant to support the original functions, and that the implementation having LOTS of duplicated code (from csbs.py and nyt.py) suggests that the current implementation can be further refactored so that "code smell" can occur less.
V regarding proper implementation V
Location class and TimelinedLocation class in init.py should be removed as it is replaced by LocationBuilderInterface and TimelinedLocationBuilderInterface respectively as the Abstract interface.
CSBSLocation class in csbs.py should be removed as it is replaced by CSBSLocationBuilder as the concrete Builder.
similarly, NYTLocation with NYTLocationBuilder in nyt.py
The Director and Product current located in csbs.py and nyt.py should be in their own respective module/file so that it is better organized and structured. (but it still works right now)
The client code at the end of csbs.py and nyt.py should belong to another module/file where the other client codes are.
The pseudo code of implementation have some lines of code commented out, which should reflect how it should looks like when properly implemented;
Those lines of code are currently commented out to prevent program from unable to compile.