文章/答案/技术大牛

发布

问Excel计算引擎
EN

Code Review用户

提问于 2014-09-29 17:03:32

回答 2查看 360关注 0票数 4

我们目前已经在F#中重写了一个Excel计算引擎，并希望重构代码，使其更加实用和标准化。

代码的一个大问题是，我们有一个名为companyModel的大型函数，它从Company类型、IntercoTimeSeries类型和DividendTrappedCashSolver类型构建了一个CompanyModel类型(包含一些缓存的嵌套函数的记录类型)。companyModel函数由许多小的嵌套函数组成(将近2,000行代码！)对输入进行计算，然后生成一个CompanyModel作为结果。

companyModel的可怕实现的一个优点是，所有嵌套函数都具有对Company、IntercoTimeSeries和DividendTrappedCashSolver输入的全局访问权，因此我们不需要将这些参数作为参数传递给所有单独的嵌套函数。这方面的问题是，代码很难单独测试，并且使用#Region注释来组织代码，而不是使用嵌套模块进行更严格的测试。

重构的第一个想法是将嵌套的#Region注释替换为嵌套模块，然后将嵌套的函数放在这些嵌套模块中。然后，companyModel函数可以调用上一个嵌套模块中的最后几个函数来计算CompanyModel结果。

然而，这方面有一些问题。其中包括：

我们失去了对Company、IntercoTimeSeries和DividendTrappedCashSolver输入的全局访问，这些输入必须在需要时通过所有函数传递。目前，F#不允许我们将全局参数传递给模块(和嵌套模块)，因此它们必须通过单个函数本身过去，从而增加参数的数量，使每个函数变得更加复杂。我们可以用类类型替换模块，因为这些类型允许我们传递全局参数，但是与模块不同的是，不能为了组织的目的而嵌套这些模块。
更复杂的是，嵌套函数中大量使用了回忆录，以便能够在以前的时间段缓存值，并减少不必要的函数重新计算。必须将这些额外的参数传递给所有函数，这可能会使回忆录计划变得复杂，而回忆录计划就是觉得不对。

什么是重构companyModel函数中嵌套函数的最佳方法，以帮助更好地进行隔离测试，但在使函数变得更复杂、参数更多和使回忆录过程复杂化方面，不改变太多的逻辑？

我在下面添加了一个companyModel函数的摘录和现有的回忆录功能。如果需要更多的细节，比如函数所使用的类型，请告诉我，但我不认为这些会影响可能的重构解决方案。

// Utility.fs
module Utility = 

    let inline memoise f = 
        let res = ref Unchecked.defaultof<_>
        let dict = System.Collections.Generic.Dictionary<int,_>(Time.timeLineLength) 
        fun v ->
            if dict.TryGetValue(v.Period, res) then
                !res
            else
                let res = f v 
                dict.Add(v.Period, res)
                res      

    let inline memoise2 f = 
        let res = ref Unchecked.defaultof<_>
        let dict = System.Collections.Generic.Dictionary<int,_>(Time.timeLineLength)
        fun u v ->
            if dict.TryGetValue(v.Period, res) then
                !res
            else
                let res = f u v 
                dict.Add(v.Period, res)
                res  

// CompanyModel.fs
module CompanyModel =

    let companyModel (c : Company, i : IntercoTimeSeries, s : DividendTrappedCashSolver) =

        // Alias Budget Model Database lookups
        let profitAndLoss = c.BudgetModelDatabase.ProfitAndLoss
        let balanceSheet = c.BudgetModelDatabase.BalanceSheet
        let cash = c.BudgetModelDatabase.Cash

        //#region - TimingFlags

        //#region -- DividendQuarter

        let dividendQuarter (t : Time) =
            let currMonth = Time.month t
            match c.DividendGroup with
            | DividendGroup.Tele -> 
                if currMonth = 3 then 4
                else if currMonth = 6 then 1
                else if currMonth = 9 then 2
                else if currMonth = 12 then 3
                else 0
            | _ -> 
                if currMonth = 2 then 4
                else if currMonth = 5 then 1
                else if currMonth = 8 then 2
                else if currMonth = 11 then 3
                else 0

        let dividendQuarterFlag t =
            dividendQuarter t <> 0

        //#endregion DividendQuarter

        //#region -- CompanyWindDown

        let companyWindDownPeriodStart =
            c.FinalMonth

        let companyWindDownFlag (t : Time) =
            t >= c.FinalMonth

        let cashflowTaxWindDownFlag (t : Time) =
            t.BudgetMonth >= c.FinalMonth.BudgetMonth - 8

        let profitAndLossTaxWindDownFlag (t : Time) =
            t.BudgetMonth >= c.FinalMonth.BudgetMonth - 12

        //#endregion CompanyWindDown

        //#region -- CompanySold

        let companySold t =
            match c.SoldMonth with
            | Some v -> t >= v
            | None -> false

        let companyNotSold t =
            match c.SoldMonth with
            | Some v -> t < v
            | None -> true

        //#endregion CompanySold

        //#endregion TimingFlags

        //#region - Opening Balances

        let openingBalanceCash = 
            memoise2 (fun closingBalanceCash (t : Time) ->
                if (t - 1).IsForecast then
                    closingBalanceCash
                else
                    balanceSheet.Cash (t - 1)
            )

        let openingBalanceIntercompanyDebt = 
            memoise2 (fun closingBalanceIntercompanyDebt (t : Time) ->
                if (t - 1).IsForecast then
                    closingBalanceIntercompanyDebt
                else
                    balanceSheet.IntercompanyLoan (t - 1) 
            )  

        let openingBalanceRetainedEarnings  = 
            memoise2 (fun closingBalanceRetainedEarnings (t : Time) ->
                if (t - 1).IsForecast then

                    closingBalanceRetainedEarnings
                else
                    - balanceSheet.RetainedEarnings (t - 1) 
            )  

        let openingBalanceShareCapital  = 
            memoise2 (fun closingBalanceShareCapital (t : Time) ->
                if (t - 1).IsForecast then
                    closingBalanceShareCapital
                else
                    - balanceSheet.ShareCapital (t - 1) 
                    - balanceSheet.SharePremium (t - 1)
            )                   

        //#endregion

        //#region - Shared Functions

        let cashBalanceInterestRate t =
            Interest.libor t + Interest.cashBalanceInterestMargin

        let cashBalanceInterestReceivable =
            memoise2 (fun closingBalanceCash (t : Time) ->
                if not (companyWindDownFlag t) then 
                    if t.IsForecast then
                        openingBalanceCash closingBalanceCash t * cashBalanceInterestRate t / 12.0
                    else
                        0.0
                else
                    0.0
            )

        let intercompanyInterestRate t =
            Interest.libor t + Interest.intercompanyInterestRate

        let intercompanyLoanInterest = 
            memoise2 (fun closingBalanceIntercompanyDebt (t : Time) ->
                if not (companyWindDownFlag (t - 1)) then
                    (openingBalanceIntercompanyDebt closingBalanceIntercompanyDebt t) * intercompanyInterestRate t / 12.0
                else
                    0.0
            )

        //#endregion

        //#region - Tax

        //#region --    Taxable Revenue Profit

        //#region ---       Taxable Revenue Profit - Before (Budget model) Interest & Capital Allowances

        let taxableRevenueProfitExcludingInterestAndCapitalAllowances (t : Time) = 

            let inline taxableProfitCheck (t : Time) f flag =
                if flag then
                    f t
                else
                    0.0

            let adjustedBonus (t : Time) =
                - profitAndLoss.Bonus t + 
                if Time.isAfterModelEnd t then
                    0.0
                else if Time.isEndOfFinancialYear t then
                    Time.midRangeSumByFast (min (t + 1) Time.modelEnd) (min (t + 12) Time.modelEnd) cash.LtisPayments
                else
                    0.0

            taxableProfitCheck t profitAndLoss.Ebt c.TaxableProfitFlags.IncludeEbtAfterIntercompanyRecharges +
            - taxableProfitCheck t profitAndLoss.AddBackDepreciationAndAmortisation c.TaxableProfitFlags.IncludeAddBackDepreciationAmortisation +
            - taxableProfitCheck t profitAndLoss.LessProfitOnDisposal c.TaxableProfitFlags.IncludeLessProfitOnDisposal +
            - taxableProfitCheck t profitAndLoss.LessSurrenderPremiums c.TaxableProfitFlags.IncludeLessSurrenderPremiums +
            - taxableProfitCheck t profitAndLoss.LessOtherNonAllowableItems c.TaxableProfitFlags.IncludeLessOtherNonAllowableItems +
            taxableProfitCheck t profitAndLoss.OnerousLeaseTaxableProfit c.TaxableProfitFlags.IncludePlusOnerousLeaseTaxableProfit +
            - taxableProfitCheck t profitAndLoss.OnerousLeaseAccountingProfit c.TaxableProfitFlags.IncludeLessOnerousLeaseAccountingProfit +
            taxableProfitCheck t profitAndLoss.IrrecoverableVatProfitAndLoss c.TaxableProfitFlags.IncludeAddIrrecoverableVat +
            taxableProfitCheck t adjustedBonus c.TaxableProfitFlags.IncludeReplaceProfitAndLossBonusWithNextYearsCashBonus

        //#endregion Taxable Revenue Profit - Before (Budget model) Interest & Capital Allowances

        //#region ---       Interest

        let intercompanyInterestReceivableForTax =

            let intercompanyLoanOpeningBalance =
                Time.lastNonZeroBy balanceSheet.IntercompanyLoan
            memoise2 (fun closingBalances (t : Time) ->

                let interCompanyInterestOnOpeningBalance (t : Time) =
                    match c.CompanyShortName with
                    | CompanyShortName.TSP -> 0.0
                    | _ -> if t.IsForecast then
                                intercompanyLoanOpeningBalance * cashBalanceInterestRate t / 12.0 
                            else 
                                0.0
                if Time.isBeforeModelStart (t - 1) then
                    0.0
                else if (t - 12).IsForecast then
                    intercompanyLoanInterest (closingBalances.IntercompanyDebt) (t - 12)
                else  
                    interCompanyInterestOnOpeningBalance (t - 1))

        let rec taxCashBalanceInterestReceivable =
            memoise2 (fun closingBalanceCash (t : Time) ->
                if Time.isAfterModelEnd (t + 1) then
                    0.0
                else if t.IsForecast then
                    cashBalanceInterestReceivable closingBalanceCash t    
                else           
                    taxCashBalanceInterestReceivable closingBalanceCash (t + 1)
            )

        let rec currentYearInterestReceivable =
            memoise2 (fun closingBalanceCash (t : Time) ->
                if Time.isAfterModelEnd (t + 11) then
                    0.0
                else if Time.isStartOfFinancialYear t then
                    Time.midRangeSumByFast t (t + 11) (taxCashBalanceInterestReceivable closingBalanceCash)
                else
                    currentYearInterestReceivable closingBalanceCash (t - 1) 
            )

        let rec assumedMonthlyInterest =
            memoise2 (fun closingBalanceCash (t : Time) ->
                if Time.isBeforeModelStart (t - 11) then
                    0.0
                else if Time.isEndOfFinancialYear t then
                    currentYearInterestReceivable closingBalanceCash t - assumedMonthlyInterest closingBalanceCash (t - 1) 
                else
                    currentYearInterestReceivable closingBalanceCash (t - 12) / 2.0 / 12.0
            )

        let cashBalanceInterestReceivableForTax closingBalanceCash (t : Time) =
            assumedMonthlyInterest closingBalanceCash (t - 12)

        //#endregion Interest

        // 1,500 more lines of business logic code here all part of the companyModel function
        // ....

excel

回答 2

Code Review用户

发布于 2014-09-29 18:05:04

companyModel的可怕实现的一个优点是，所有嵌套函数都具有对公司、IntercoTimeSeries和DividendTrappedCashSolver输入的全局访问权，因此我们不需要通过所有单独的嵌套函数传递这些参数。

正如神话破坏者们喜欢说的那样，“好吧，这是你的问题。”这种说法直接违反了依赖反演原则--依赖于抽象，而不是具体。正如您在下一句中提到的，您已经意识到这使您的代码不可测试。这绝不是“积极的”。

尝试重构代码以传递所需的依赖项。这将使您的逻辑在该函数中成为无状态的。这将使您能够为您的逻辑编写测试。

您将不得不一次攻击一个小模块或函数。我首先从“最内部”的功能层开始，一个依赖项最少的层，它生成其他函数所依赖的值。然后，当您为使用此值的模块编写测试时，您不会尝试测试所有内部函数是否正确，因为您已经对它们进行了测试。

这听起来可能是件大事，但你说只有2000行。如果它扩展到4000条线或8000条线，那又如何？这不像额外消耗几千字节的储藏室会破坏银行；你也不会处理一堆打孔卡。较小的程序大小并不是质量的指标(除非您正在编写Mars，一个用于设备的嵌入式模块，或者是一些具有72帧每秒刷新速率的第一人称射击游戏)。但是，拥有不可维护、不可测试的代码可能会给您的业务带来灾难性的后果。这比代码大小重要得多。

票数 2

Code Review用户

发布于 2014-11-06 15:14:09

虽然这并不直接解决如何通过重构来解决更大的问题，但是减少代码的数量和复杂性应该会使这样做更简单。下面是一些关于如何做到这一点的建议(我手头没有F#编译器，所以请原谅任何错误)。

我会改变

let dividendQuarter (t : Time) =
    let currMonth = Time.month t
    match c.DividendGroup with
    | DividendGroup.Tele -> 
        if currMonth = 3 then 4
        else if currMonth = 6 then 1
        else if currMonth = 9 then 2
        else if currMonth = 12 then 3
        else 0
    | _ -> 
        if currMonth = 2 then 4
        else if currMonth = 5 then 1
        else if currMonth = 8 then 2
        else if currMonth = 11 then 3
        else 0

至

let dividendQuarter (t : Time) =
    match c.DividendGroup with
    | DividendGroup.Tele ->
        match Time.month t with
        | 3 -> 4
        | 6 -> 1
        | 9 -> 2
        | 12 -> 3
        | _ -> 0
    | _ ->
        match Time.month t with
        | 2 -> 4
        | 5 -> 1
        | 8 -> 2
        | 11 -> 3
        | _ -> 0

或更新:添加(DividendGroup.Tele, _) -> 0以匹配原始实现

let dividendQuarter (t : Time) =
    match (c.DividendGroup, Time.month t) with
    | (DividendGroup.Tele, 3) -> 4
    | (DividendGroup.Tele, 6) -> 1
    | (DividendGroup.Tele, 9) -> 2
    | (DividendGroup.Tele, 12) -> 3
    | (DividendGroup.Tele, _) -> 0
    | (_, 2) -> 4
    | (_, 5) -> 1
    | (_, 8) -> 2
    | (_, 11) -> 3
    | _  -> 0

根据口味。

您有许多具有相同模式的函数，例如：

 let openingBalanceShareCapital  = 
            memoise2 (fun closingBalanceShareCapital (t : Time) ->
                if (t - 1).IsForecast then
                    closingBalanceShareCapital
                else
                    - balanceSheet.ShareCapital (t - 1) 
                    - balanceSheet.SharePremium (t - 1)
            )

我会把它们改为类似的东西(也许这个想法也可以应用于其他领域)：

let opening balance t =
    let transformed =
        memoise2 (fun closing (t : Time) ->
            if (t - 1).IsForecast then
                closing
            else
                balance t
        )
    transformed

let balanceCash t =
    balanceSheet.Cash (t - 1)

let alanceIntercompanyDebt t =
    balanceSheet.IntercompanyLoan (t - 1) 

let alanceRetainedEarnings t =
    - balanceSheet.RetainedEarnings (t - 1) 

let balanceShareCapital t =
    - balanceSheet.ShareCapital (t - 1) - balanceSheet.SharePremium (t - 1)

let openingBalanceCash = opening balanceCash t
let openingBalanceIntercompanyDebt = opening alanceIntercompanyDebt t
let openingBalanceRetainedEarnings = opening alanceRetainedEarnings t
let openingBalanceShareCapital = opening balanceShareCapital t

关于Company、IntercoTimeSeries和DividendTrappedCashSolver，您可以将它们放在一个记录中，从而减少必须传递的参数的数量。

票数 2

页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持

原文链接：

https://codereview.stackexchange.com/questions/64168

复制

相似问题

问Excel计算引擎
EN

回答 2

Code Review用户

Code Review用户

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问Excel计算引擎EN

回答 2

Code Review用户

Code Review用户

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问Excel计算引擎
EN