Skip to content
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

Alternative design #1

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Alternative design #1

wants to merge 1 commit into from

Conversation

neoeno
Copy link

@neoeno neoeno commented Aug 23, 2023

Hi Will,

Thought I'd draw over your design a little to show another potential path.

This one reduces some of the design complexity around inheritance, calls within
the body of the class, removes a few classes, etc. It does add a central registry
list in GildedRose which would be open for modification, but ultimately you have
a list of the files you're requiring in there anyway so it doesn't meaningfully
decrease the open-for-modification-ness of the overall design.

This design also enables the caller of GildedRose to control the list of updaters
and their order — a nice design feature that increase the reusability of the class.
I would tempted to have the caller of GildedRose always have to specify either
their own list or pass the DEFAULT_UPDATERS list explicitly, but that does
definitely increase Connascence between the callers
and the Gilded Rose class so I decided against it in the end.

Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant