Excel calculation engine

We have currently rewritten an Excel calculation engine in F# and are looking to refactor the code to make it a lot more idiomatic and standardised.

One of the big problems with the code is that we have a massive function named companyModel that builds a CompanyModel type (a record type that holds some of the cached nested functions) from a Company type, IntercoTimeSeries type and DividendTrappedCashSolver type. The companyModel function consists of lots of small nested functions (nearly 2,000 lines of code!) that do calculations with the inputs and then produce a CompanyModel as a result.

One of the positives with the horrific implementation of companyModel is that all the nested functions have global access to the Company, IntercoTimeSeries and DividendTrappedCashSolver inputs so we don't need to pass these as arguments through all our individual nested functions. The issues with this are that the code is hard to test in isolation and is softly organised using #Region comments instead of say more rigidly using nested modules.

The first idea for refactoring was to replace the nested #Region comments with nested modules and to then put the nested functions in these nested modules.
The companyModel function could then just call the last few functions in the last nested module to calculate a CompanyModel result.

However there are a few problems with this. These include:

• We lose global access to Company, IntercoTimeSeries and

DividendTrappedCashSolver inputs and these then have to be passed
though all functions where required. Currently F# doesn't allow us
to pass global parameters to modules (and nested modules) hence they
must be past via the individual functions themselves and hence
increasing the number of parameters and making each function more
complex. We could maybe replace the modules with class types as
these do let us pass global parameters, however unlike modules these
can't be nested for organisation purposes.

• To

One of the positives with the horrific implementation of companyModel
is that all the nested functions have global access to the Company,
IntercoTimeSeries and DividendTrappedCashSolver inputs so we don't
need to pass these as arguments through all our individual nested
functions.

As the Mythbusters are fond of saying, "Well, there's your problem." This statement is directly contrary to the Dependency Inversion Principle -- depend on abstractions, not on concretions. As you mention in the very next sentence, you already realize this renders your code untestable. It is in no way a "positive".

Try refactoring your code to pass in the needed dependencies. This will have the effect of making your logic in that function stateless. That will enable you to write tests for your logic.

You'll have to attack one small module or function at a time. I'd start with the "innermost" layer of functionality, one with the smallest number of dependencies, and which produces a value that other functions depend upon. Then as you write tests for the modules that use this value, you're not trying to test that all the inner functions are correct, because you've already tested them.

It may sound like a big deal, but you said it's only 2000 lines. If it expands to 4000 lines or 8000 lines, so what? It's not like consuming a few extra kilobytes of storage are going to break the bank; and you're not dealing with decks of punched cards. A smaller program size is not an indicator of quality (unless you're programming the Mars Rover, an embedded module for an appliance, or some first person shooter game with a 72 frames per second refresh rate.) However, having unmaintainable, untestable code could yield disastrous results for your business. That's a lot more important than code size.

StackExchange Code Review Q#64168, answer score: 2