Skip to content

Add ReturnOnInvestment to maths#7446

Open
InukaWijerathna wants to merge 5 commits into
TheAlgorithms:masterfrom
InukaWijerathna:add/return-on-investment
Open

Add ReturnOnInvestment to maths#7446
InukaWijerathna wants to merge 5 commits into
TheAlgorithms:masterfrom
InukaWijerathna:add/return-on-investment

Conversation

@InukaWijerathna

Copy link
Copy Markdown

Description

Adds ReturnOnInvestment to the maths package.

Return on Investment (ROI) is a fundamental financial metric that quantifies how profitable an investment is relative to its cost:

ROI = (Gain from Investment - Cost of Investment) / Cost of Investment × 100

What's included

  • ReturnOnInvestment.java — clean implementation with Javadoc and IllegalArgumentException for invalid cost
  • ReturnOnInvestmentTest.java — 6 JUnit 5 tests covering positive, zero, and negative ROI, plus error paths

Checklist

  • Follows the code style of the project (final class, private constructor, static method)
  • JUnit 5 tests for all branches including exception cases
  • Javadoc with formula and reference link

Reference: https://www.investopedia.com/terms/r/returnoninvestment.asp

@codecov-commenter

codecov-commenter commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.72%. Comparing base (cecfad9) to head (2bced69).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #7446      +/-   ##
============================================
- Coverage     79.72%   79.72%   -0.01%     
- Complexity     7298     7300       +2     
============================================
  Files           803      804       +1     
  Lines         23762    23771       +9     
  Branches       4674     4675       +1     
============================================
+ Hits          18945    18952       +7     
  Misses         4059     4059              
- Partials        758      760       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@DenizAltunkapan

Copy link
Copy Markdown
Member

Thanks for the contribution. The implementation itself is correct and the tests cover the basic cases well.

However, I am not fully convinced that this belongs in TheAlgorithms/Java as an algorithm. This is essentially a direct one-line financial formula rather than an algorithmic implementation. Accepting this may also set a precedent for many similar financial-ratio helper methods.

I would not approve this as-is...

@InukaWijerathna

Copy link
Copy Markdown
Author

Thanks for the feedback! I've expanded the implementation to address the concern. The class now includes annualizedReturnOnInvestment(), which converts a total ROI over a multi-year holding period into an equivalent annual rate using the geometric-mean formula:

Annualized ROI = ((1 + ROI/100)^(1/n) - 1) x 100

This chains the simple ROI calculation, applies Math.pow with a fractional exponent, and validates an additional parameter (holding period in years). I've also added 8 new tests covering the one-year identity case, multi-year, fractional years, break-even, negative ROI, and invalid inputs. Happy to adjust further if needed!

Extends the ROI class with annualizedReturnOnInvestment(), which applies
the geometric-mean formula to convert a total ROI over n years into an
equivalent annual rate. Adds 8 new tests covering one-year identity,
multi-year, fractional-year, break-even, negative ROI, and invalid inputs.
@InukaWijerathna InukaWijerathna force-pushed the add/return-on-investment branch from 7bee01c to 2bced69 Compare June 6, 2026 13:05
@InukaWijerathna

Copy link
Copy Markdown
Author

Hi! Just checking in to see if there's any update on this PR — happy to make any changes needed to get it merged. Thanks for your time!

@DenizAltunkapan DenizAltunkapan enabled auto-merge (squash) June 9, 2026 14:59
@DenizAltunkapan DenizAltunkapan disabled auto-merge June 9, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants