Skip to content

Add money pattern #326

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

Closed
wants to merge 3 commits into from
Closed

Conversation

carousel
Copy link
Contributor

Hi folks, I have added famous money pattern to the more folder. It is tested and I have cleaned code based on my previous mr. If you notice some issues please let me know.

@domnikl
Copy link
Contributor

domnikl commented May 26, 2018

Hi @carousel. First of all, thanks for the PR! I wouldn't call it a Money pattern but rather it shows the usage of value objects by the example of a Money class. I would see that more in the "SOLID + Value objects + other basic design practices" department than in the "Design Patterns" department. While I think it would fit nicely in a format like this (samples in a git repo on Github) it doesn't fit in here.

@piotrgradzinski what's your opinion about this?

@fredlawl
Copy link

fredlawl commented May 26, 2018

Afternoon @carousel. FWIW I agree that this is very representative of a value object. There's something to improve as well. Each currency that I know of can be broken down into fractional parts. I would like to see a money class that handles floating point number rounding errors. It's easier in languages that have a type that handles that situation, and unfortunately that's not the case in PHP.

@piotrgradzinski
Copy link
Contributor

@domnikl - I agree with you. We should not think of this repository as a place for every possible pattern, approach, best practise. This is good to have it clean and related only to design patterns.

I do really appreciate @carousel effort, and this is great! Nevertheless, as @domnikl said, it not really fits.

@carousel
Copy link
Contributor Author

Hi @domnikl @piotrgradzinski @fredlawl tnx for respone,
You are all right if we observe patterns from the strict GoF perspective. My motivation for including this pattern was because in many design patterns repos here on github, there is usually additional folder with other patterns like null object, DTO ...
Money pattern is finding its way into patterns because it brings some of the fundamental concepts like immutability, pass by value, invariant guard ... But obviously not into this collection.
Anyway tnx. I am closing mr.

@carousel carousel closed this May 31, 2018
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.

4 participants