Rremove unnecessary ratio validation logic.

[tabennett]

• Removed ratio validation logic that checks that the sum of ratios is non-negative.
• Removed ratio validation logic that prevented negative ratios.

Both of these methods currently block valid use cases for the allocate method. In general, it should always be possible to allocate ratios as long as there is at least one non-negative ratio. Guarding against this invariant should not be the responsibility of the money object.

[frederikbosch]

@sagikazarmark I am OK with merging this. What is up with the psalm analysis? I only use PHPStan on other projects, so I cannot tell.

[tabennett]

I'm not sure what's going on there. I don't use psalm either. I was hoping you guys would know.

[frederikbosch]

Will look at this again after #634 is merged.

[tabennett]

@frederikbosch Is there a specific reason why? That branch doesn't look like it's going to be ready to release anytime soon and I PR'd this over 3 months ago (and that was after first explaining this issue and being asked if I could PR it).

634 Doesn't fix this issue. It's still throwing the same exception here.

[frederikbosch]

@tabennett Would you want to have a look at this again? What do you do when the total of the ratios is zero? Suppose you want to allocate 10 to 0, -1 and 1. What do you expect the outcome to be? I will only consider this when there is no BC break and working tests. This PR is closed until the mentioned criteria are met.

[tabennett]

So I've been thinking about this. It looks like removing this check should be all that's necessary.

[frederikbosch]

@tabennett Well yes. But can you provide (unit / spec) tests which proves that the system calculates the correct answer. Again, suppose you have new Money(10, new Currency('EUR')). And you call $money->allocate([0, -1, 1]), what do you expect the outcome to be?

[tabennett]

@frederikbosch I expect that php Money::EUR(10)->allocate([0, -1, 1]) throws an exception before the suggested changes that I would PR are even executed.

I'm suggesting removal of lines 301 through 302 only. This change would have no affect on the example you're asking me about...

[frederikbosch]

Well, in the PR you are suggesting more lines to be removed. But suppose you want only these specific lines to be removed, then I am wondering what the outcome of $money->allocate([0, -1, 4]) would be. Removing those lines would allow negative ratios while the total still must be higher than zero, as in the example.

If you can explain me a scenario, the logic why that outcome is logic and we can capture that in a test, then I am happy to merge that.