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

tests: PortfolioBuilder methods accepting "real" number of shares/amounts? #4481

Open
pfalcon opened this issue Jan 18, 2025 · 0 comments
Open

Comments

@pfalcon
Copy link
Contributor

pfalcon commented Jan 18, 2025

@buchen :

Continuing #4480 on how to make test writing easier, IMHO PP lacks in the area of unit tests. Many tests are in fact integration ones, starting from an XML file. That's however not ideal for all cases, because:

  • it's unclear what's inside XML, requires opening in PP to just see that well
  • it's hard to vary testcases a bit (how to do that - create a bunch of XML files, and then maintain them).

So IMHO, there should be improvement to writing unit tests for various core functionality, to increase coverage.

In that regard, it would be nice to have methods where you say "I want to buy 2 stocks, amounted 100 money), and don't bother with explicit fixed-point conversions as tests seem to do now. I started to add overloads like:

+    public PortfolioBuilder buy(Security security, String date, double shares, double amount, double fees, double taxes)
+    {
+        return buy(security, date, sharesOf(shares), amountOf(amount), amountOf(fees), amountOf(taxes));
+    }

in addition to existing long (i.e. fixed-point) taking methods. Would that be ok? Having them as overloads is of course risky if both variants are kept used widely, then it's easy to make mistake (and outright confusing to new users) depending on just the type. So, IMHO it would only work well if there's effort to refactor existing tests to use double variant, and then perhaps rename the fixed-point versions as buyFixPt(), etc.

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

No branches or pull requests

1 participant