search

fsprojects / ExcelFinancialFunctions

.NET Standard library providing the full set of financial functions from Excel.
https://fsprojects.github.io/ExcelFinancialFunctions
Other
195 stars 66 forks source link

Remove incorrect checks for PMT and FV calculations #67

Closed evanknapke closed 2 years ago

evanknapke commented 2 years ago

It is perfectly valid for FV and PV to both be zero in the PMT calculations. It is perfectly valid for PMT and PV to both be zero in the FV calculations.

I have only fixed these, but it seems there are some other unnecessary checks like this in some of the other calculations that would be worth looking over.

jcoliz commented 2 years ago

Thanks for submitting this. I will have a look!

jcoliz commented 2 years ago

Looks good. Just waiting for the long-running "console" tests to finish. Also, I like for new code to have dedicated tests in spottests.fs, so I'll add those also.

@evanknapke Question for you... Why? Is there every any practical reason to call these functions in the way described here? The result can only be $0. You're correct that Excel allows it, so the library should too. Still, I'm curious what led you here?

Finally, do you need new packages released to NuGet Gallery? I can do so if you need it.

evanknapke commented 2 years ago

Looks good. Just waiting for the long-running "console" tests to finish. Also, I like for new code to have dedicated tests in spottests.fs, so I'll add those also.

@evanknapke Question for you... Why? Is there every any practical reason to call these functions in the way described here? The result can only be $0. You're correct that Excel allows it, so the library should too. Still, I'm curious what led you here?

Finally, do you need new packages released to NuGet Gallery? I can do so if you need it.

@jcoliz Sure thing... We are using this library to run several calculations based off of a lot of user input. Sometimes the user may input data that leads to this part of the calculations to have 0 values for pmt, pv, and/or fv. If this is the case, exceptions were thrown and broke these parts of the process. We specifically noticed it in the FV and PMT functions, which is why these were the only ones I fixed in this Pull Request.

So, I agree that it does not make much sense to call these functions since it will always be $0, but it is still valid to do so and lead me to make the PR.

It would be great if you could make a release to NuGet if possible with these changes!

jcoliz commented 2 years ago

@evanknapke Sure thing. I have published it to the NuGet Gallery. It's indexing now, will be available soon. Please feel free to submit PRs for any other similar problems. Bonus points for checking in a failing unit test first (e.g. 20ff52e05a75cf463abc2caedcf7990e7886f2e5) in a separate commit.

Thanks for your contribution!