search

moneyphp / money

PHP implementation of Fowler's Money pattern.
http://moneyphp.org
MIT License
4.6k stars 440 forks source link

introduce Teller class #630

Closed pmjones closed 1 year ago

pmjones commented 3 years ago

This PR introduces a Teller class, nominally to help ease legacy codebases away from float math over to using Money object.

It differs from the tentative offering in #629 in several ways, most notably that it is PHP 5.6 compatible, and reproduces a larger number of Money methods, using their same names.

I have attempted to match the existing project conventions; please let me know if changes of any kind are needed.

Tests and docs are included.

fixes #629

pmjones commented 3 years ago

After making several changes to soothe the CI checks, all remaining failures appear to be problem with the build system, not with the PR.

lukeholder commented 1 year ago

Is this planned to be merged?

frederikbosch commented 1 year ago

We should rebase this towards current master.

lukeholder commented 1 year ago

@frederikbosch looks like it’s merged

pmjones commented 1 year ago

@frederikbosch @lukeholder I have updated to master. However, a problem is revealed in testing.

Previously, I used the PhpCalculator for the Teller tests, and the it_multiplies_amounts and it_divides_amounts tests passed; however, using the default calculator, they do not -- the multiplication and division appear to be off. (Running the math of the tests through something like a spreadsheet indicates the expected values are correct.)

How to proceed?

frederikbosch commented 1 year ago

Great work @pmjones, and thanks for the quick response, especially after us not giving this time in the first place.

pmjones commented 1 year ago

@frederikbosch I will make the changes as noted. Shall I also add property typehints?

pmjones commented 1 year ago

@frederikbosch All corrections made, and added some more typehinting to soothe Psalm (though it still complains a bit).

frederikbosch commented 1 year ago

If you can allow edits from maintainers, I am happy to commit some changes.

pmjones commented 1 year ago

I have sent you a "collaborator" invitation!

frederikbosch commented 1 year ago

All checks are green. The class is now final, as all classes in this package are I believe. Otherwise, mostly code style and docblock changes.

frederikbosch commented 1 year ago

@pmjones If you believe this is good to go, we can merge it in.

pmjones commented 1 year ago

@frederikbosch Send it!

frederikbosch commented 1 year ago

Thanks for the work @pmjones. After quite some time, but it did finally get included in the package!

pmjones commented 1 year ago

@frederikbosch I know how these things go -- everyone is working on their own time. Thanks for getting to it at all!

lukeholder commented 1 year ago

Has this been tagged in a release?

frederikbosch commented 1 year ago

Not yet, don't know why not. I'll tag today.

frederikbosch commented 1 year ago

Done!

pmjones commented 1 year ago

Thanks @frederikbosch ! (Not to be a nag, but the moneyphp.org site probably needs the updated docs for Teller as well.)

frederikbosch commented 1 year ago

In all these years that I am doing this, I think I have maybe once logged in there. I will try to fix this, but it will take some time to figure how this works, and how to gain access. I always believed that we automated this.

pmjones commented 1 year ago

/me nods

No rush on my part, and thanks again!