moic returns a string instead of NaN

[KuldeepBorkar]

When no Positive or Negative values are provided to the function moic it will return a string that No Positive or Negative values are Provided instead of returning NaN

[codecov]

Codecov Report

Merging #92 (39ccaa4) into master (bf6e13b) will decrease coverage by 0.45%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   97.97%   97.52%   -0.46%     
==========================================
  Files           4        4              
  Lines         198      202       +4     
==========================================
+ Hits          194      197       +3     
- Misses          4        5       +1     

Flag	Coverage Δ
unittests	97.52% <75.00%> (-0.46%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/financial_math.jl	96.75% <75.00%> (-0.59%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[alecloudenback]

It's a good idea to help users with a more informative error here!

Three suggestions/comments:

1. In Julia it's better to not return different types (string and number) if possible. This helps Julia optimize because the compiler doesn't have to contemplate handing both strings and numbers.
2. I think something like all( cf -> sign(cf) == sign(first(cfs)), cfs) would be a lot more performant (you can double check with BenchmarkTools.jl) because it will avoid allocating the pos and neg arrays
3. Please add a test case for the new error functionality.

[KuldeepBorkar]

@alecloudenback all( cf -> sign(cf) == sign(first(cfs)), cfs) I am not getting it ; ( how this would work because if I used it doing something like this:

function moic(cfs::T) where {T<:AbstractArray}
    if all(cf -> sign(cf) == sign(first(cfs)), cfs)
        returned = sum(cf for cf in cfs if cf > 0)
        invested = -sum(cf for cf in cfs if cf < 0)
        return returned / invested
    else
        return "No +ve or -ve mentioned in input"
    end
end

Then even if in input someone mentioned

moic([0,0,0,0])

Then also it will return NaN only

[alecloudenback]

Hi @KuldeepBorkar,

After a lot of thought and discussion here, I would favor not throwing an error or special casing the moic function as you proposed in your initial commit. The reason is that floating point math is designed to work the way it does for very well-thought-out reasons. And a pattern of cashflows that is all positive, negative, or all zeros is not an unreasonable input, so it's not really an argument error either. Throwing an error would halt the program when there may be no issue to warrant stopping the program, and we don't want to return a string sometimes and a number of other times because we are implicitly asking for a number to be the result of calling moic. Does this make sense?

I think there are two improvements to moic that would still be really appreciated:

1. Indicating in the docstring that the math could result in an infinite or undefined MOIC if there are not opposite signed cashflows in the input.
2. Improving the performance of the function using the strategy suggested in this post