Net Present Value (NPV) added. #285
Conversation
WalkthroughAdds Net Present Value (NPV) functionality to the valuation package, including implementation and tests. Updates documentation: root README adds NPV entry; valuation README documents Npv with formula and placement; helper README documents several helper functions. No existing public APIs are modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client as Caller
participant Val as valuation.Npv
participant Math as math.Pow
Client->>Val: Npv(rate, cfs)
loop for each cf at index i
Val->>Math: Pow(1+rate, i+1)
Math-->>Val: discountFactor
Val-->>Val: npv += cf / discountFactor
end
Val-->>Client: npv
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #285 +/- ##
=======================================
Coverage 91.79% 91.79%
=======================================
Files 191 191
Lines 6797 6797
=======================================
Hits 6239 6239
Misses 494 494
Partials 64 64 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
valuation/README.md (1)
29-41: Clarify CF indexing and CF0 usage in NPV docsImplementation discounts flows starting at period 1. Please state that cfs represents periods 1..n, and to include an initial t=0 cash flow (CF0) users should add it separately: CF0 + Npv(rate, cfs). Since this file is generated, add this wording to the Npv doc comment in valuation/npv.go so gomarkdoc picks it up.
valuation/npv.go (1)
9-12: Document CF0 handling and preconditionsAdd brief notes about period indexing and disallowing rate == -1 to avoid div-by-zero.
Apply this diff to the Npv doc comment:
// Npv calculates the Net Present Value (NPV) of a series of cash flows. // -// Formula: NPV = sum(CF_i / (1 + rate)^i) for i = 1 to n +// Formula: NPV = sum(CF_i / (1 + rate)^i) for i = 1 to n +// Notes: +// - cfs contains period-1..n cash flows. If you have an initial t=0 cash +// flow (CF0), include it separately: NPV_total = CF0 + Npv(rate, cfs). +// - Preconditions: 1 + rate must not be 0 (i.e., rate != -1).valuation/npv_test.go (2)
7-14: Don’t ignore parse errors; split on whitespaceSplit with strings.Fields to avoid empty tokens, and surface parse errors so bad test data doesn’t pass silently.
Apply this diff:
import ( + "fmt" "strconv" "strings" "testing" "github.com/cinar/indicator/v2/helper" "github.com/cinar/indicator/v2/valuation" ) func TestNpv(t *testing.T) { type NpvData struct { Rate float64 CashFlows string NPV float64 } - parseCashFlows := func(s string) []float64 { - var cashFlows []float64 - for _, cfStr := range strings.Split(s, " ") { - cf, _ := strconv.ParseFloat(cfStr, 64) - cashFlows = append(cashFlows, cf) - } - return cashFlows - } + parseCashFlows := func(s string) ([]float64, error) { + var cashFlows []float64 + for _, cfStr := range strings.Fields(s) { + cf, err := strconv.ParseFloat(cfStr, 64) + if err != nil { + return nil, fmt.Errorf("parse cash flow %q: %w", cfStr, err) + } + cashFlows = append(cashFlows, cf) + } + return cashFlows, nil + } input, err := helper.ReadFromCsvFile[NpvData]("testdata/npv.csv") if err != nil { t.Fatal(err) } for row := range input { - cashFlows := parseCashFlows(row.CashFlows) + cashFlows, err := parseCashFlows(row.CashFlows) + if err != nil { + t.Fatal(err) + } npv := helper.RoundDigit(valuation.Npv(row.Rate, cashFlows), 2) if npv != row.NPV { t.Fatalf("actual %v expected %v", npv, row.NPV) } } }Also applies to: 23-30, 37-41
41-41: Optional: keep testing after first mismatchUse t.Errorf to report all failing rows in one run.
Apply this diff:
- t.Fatalf("actual %v expected %v", npv, row.NPV) + t.Errorf("actual %v expected %v", npv, row.NPV)helper/README.md (2)
687-695: Nit: file name casing appears inconsistentLink shows helper/Lowest.go (capital L). Go source files are typically lowercase; consider standardizing in code and docs in a follow-up to avoid churn on case-sensitive filesystems. Generated docs will follow the source filename.
1094-1102: Nit: tighten Window note phrasingSince this is generated, adjust the comment in helper/window.go so gomarkdoc outputs something like:
“Note: elems in the window slice keep source order. The first logical index may not be 0; use modulo w if positional alignment matters.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
valuation/testdata/npv.csvis excluded by!**/*.csv
📒 Files selected for processing (5)
README.md(1 hunks)helper/README.md(7 hunks)valuation/README.md(2 hunks)valuation/npv.go(1 hunks)valuation/npv_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
valuation/npv_test.go (2)
helper/csv.go (1)
ReadFromCsvFile(310-317)valuation/npv.go (1)
Npv(12-19)
🔇 Additional comments (4)
README.md (1)
109-109: Add NPV entry — LGTMThe new link and placement under Asset Valuation look correct.
valuation/README.md (1)
12-12: Index entry for Npv — LGTMAnchor and ordering match the implementation.
helper/README.md (2)
57-57: Index additions — LGTMNew entries (Highest, Lowest, MaxSince, MinSince, SlicesReverse, Window) are correctly indexed.
Also applies to: 65-65, 68-69, 88-88, 93-93
591-599: Docs read well — LGTMSummaries for Highest, MaxSince/MinSince, and SlicesReverse are clear and consistent.
Also applies to: 730-747, 1032-1039
| func Npv(rate float64, cfs []float64) float64 { | ||
| var npv float64 | ||
| for i, cf := range cfs { | ||
| npv += cf / math.Pow(1+rate, float64(i+1)) | ||
| } | ||
|
|
||
| return npv | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid O(n) math.Pow calls; guard rate == -1
Use a running discount factor for O(1) per element and handle the division-by-zero edge case.
Apply this diff:
func Npv(rate float64, cfs []float64) float64 {
var npv float64
- for i, cf := range cfs {
- npv += cf / math.Pow(1+rate, float64(i+1))
- }
-
- return npv
+ // Guard: 1+rate must not be 0 (division by zero at first term).
+ if rate == -1 {
+ return math.NaN()
+ }
+ if len(cfs) == 0 {
+ return 0
+ }
+ disc := 1 + rate
+ denom := disc
+ for _, cf := range cfs {
+ npv += cf / denom
+ denom *= disc
+ }
+ return npv
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func Npv(rate float64, cfs []float64) float64 { | |
| var npv float64 | |
| for i, cf := range cfs { | |
| npv += cf / math.Pow(1+rate, float64(i+1)) | |
| } | |
| return npv | |
| } | |
| func Npv(rate float64, cfs []float64) float64 { | |
| var npv float64 | |
| // Guard: 1+rate must not be 0 (division by zero at first term). | |
| if rate == -1 { | |
| return math.NaN() | |
| } | |
| if len(cfs) == 0 { | |
| return 0 | |
| } | |
| disc := 1 + rate | |
| denom := disc | |
| for _, cf := range cfs { | |
| npv += cf / denom | |
| denom *= disc | |
| } | |
| return npv | |
| } |
🤖 Prompt for AI Agents
In valuation/npv.go around lines 12 to 19, replace the per-iteration math.Pow
usage with a running discount factor to avoid O(n) pow calls and add a guard for
rate == -1 to prevent division-by-zero: check if rate == -1 at the top and
return math.NaN() (or another sentinel you prefer), otherwise initialize
discount := 1.0 + rate and in the loop divide cf by discount and then multiply
discount by (1.0 + rate) for the next iteration (or update discount
incrementally before/after use to match the current period), ensuring you import
math if needed.
Describe Request
Net Present Value (NPV) added.
Change Type
Valuation algorithm.
Summary by CodeRabbit
New Features
Documentation
Tests