finmath / finmath-lib

Mathematical Finance Library: Algorithms and methodologies related to mathematical finance.
Apache License 2.0
488 stars 168 forks source link

Code refactoring #91

Open alensden2 opened 1 year ago

alensden2 commented 1 year ago

Set #1

  1. Explaining Variable
    Location - src\main\java\net\finmath\fouriermethod\calibration\models\CalibratableMertonModel.java method name - getCharacteristicFunctionModel() Prev commit - 93620a2753b96db5130b2d55bbb0df62ccba5e4b This commit - 399f2451c1ca527571d2d15ac019dc458d819571 Maven compile status - success Line : 104 Explanation : In this refactored code, each field of the descriptor object is assigned to an explaining variable with a descriptive name. The method then returns a new MertonModel object constructed using these variables. The refactoring has improved the readability and understandability of the code.

  2. Decompose Conditional Location - src\main\java\net\finmath\marketdata\products\MarketForwardRateAgreement.java method - getValue() Prev commit - 399f2451c1ca527571d2d15ac019dc458d819571 This commit - a0ff0fbfcc72ceed1ffb8afc6131af59cb27c758 Maven compile status - success Line : 62 Explaination : While running Designite - it picked up a complex conditional smell - “if(forwardCurve == null && forwardCurveName != null && forwardCurveName.length() > 0)” This was refactored by adding another layer of abstraction and encapsulating the if conditional logic, to another method - “ public boolean checkForwardCurve(ForwardCurve forwardCurve, String forwardCurveName){ return (forwardCurve == null && forwardCurveName != null && forwardCurveName.length() > 0); } ” Now the if becomes - if(checkForwardCurve(forwardCurve, forwardCurveName))

  3. Rename method/variable Location : src\main\java\net\finmath\timeseries\models\parametric\DisplacedLognormalARMAGARCH.java method : getLogLikelihoodForParameters() Prev commit : a0ff0fbfcc72ceed1ffb8afc6131af59cb27c758 This commit : 9535176d12c06d76ddab77a949c065b884921561 Maven compile status : success Line : 92 Explanation : In this version, variables like evalPrev, eval, value1, and value2 have been renamed to evaluationPrev, evaluation, currentValue, and nextValue, respectively, to make their purpose more clear.

    Set #2

  4. Extract Class Location : src\main\java\net\finmath\marketdata\calibration\CalibratedCurves.java method : createDiscountCurve() Prev commit : 9535176d12c06d76ddab77a949c065b884921561 This commit : 0ab8b66e5c1725e093eba433f10b421870678475 Maven compile status : success Line : 758 Explanation : the class CalibratedCurves had a feature envy problem due to the fact that this class had multiple responsibilities assigned to it, Here the class had the responsibility of creating calibrated curves and discounted analytic curves.By following the "Extract Class" refactoring. I have moved the method createDiscountCurve to a new class DiscountedAnalyticCurves and made it a public method.

By doing this, I have improved the code's readability and maintainability. The code now has a clear separation of concerns, and the CalibratedCurves class no longer has a feature envy issue. The DiscountedAnalyticCurves class now has a single responsibility of creating discount curves from an analytic model.

Additionally, I have reduced the coupling between the CalibratedCurves and AnalyticModel classes by passing the AnalyticModel as a parameter to the createDiscountCurve method instead of accessing it through a field. This makes the DiscountedAnalyticCurves class more reusable and testable.

New added class Location : src\main\java\net\finmath\marketdata\calibration\DiscountedAnalyticCurves.java

  1. Replace condition with polymorphism Location : src\main\java\net\finmath\interpolation\RationalFunctionInterpolation.java method - getValue() Prev commit - 0ab8b66e5c1725e093eba433f10b421870678475 This commit - ea7248cbe969580226eb5c0061106286c736f2e5 Maven compile status - success Explanation - To replace the conditional with polymorphism, I identified the common behaviour between the different types of extrapolation methods and then create a superclass that contains this behaviour. Then, I create a subclass for each type of extrapolation method and move the behaviour specific to each method to its respective subclass. Created an abstract superclass Extrapolation: location - src\main\java\net\finmath\interpolation\Extrapolation.java

Created a subclass for each type of extrapolation method and implement the specific behaviour in each subclass: Location - src\main\java\net\finmath\interpolation\LinearExtrapolation.java Location - src\main\java\net\finmath\interpolation\ConstantExtrapolation.java

With this refactored code, we have replaced the conditional logic for extrapolation with a polymorphic approach, making the code easier to understand and maintain.

  1. Move method Location : src\main\java\net\finmath\montecarlo\GammaProcess.java method : getIncrement() line: 118 Moved method to : src\main\java\net\finmath\montecarlo\RandomVariableLazyEvaluation.java line: 111 Prev commit - ea7248cbe969580226eb5c0061106286c736f2e5 This commit - 64f8845e913bbd9f600ee922092364d5b43f4eac Maven compile status - success Explanation - The Gamma method had getIncreament method in it which was returning a random variable, It also follows lazy initialization and there was already a class for randomVariablesLazyEvaluation. I moved the method to that class to ensure better code maintainability and readablity.

    ALL COMMITS : commit 64f8845e913bbd9f600ee922092364d5b43f4eac (HEAD -> code-refactoring) Author: Alen John al283652@dal.ca Date: Wed Apr 5 21:36:23 2023 -0300

    Move Method -Set 2

commit ea7248cbe969580226eb5c0061106286c736f2e5 Author: Alen John al283652@dal.ca Date: Wed Apr 5 18:07:56 2023 -0300

Replace condition with polymorphism -Set 2

commit 0ab8b66e5c1725e093eba433f10b421870678475 Author: Alen John al283652@dal.ca Date: Wed Apr 5 16:31:25 2023 -0300

Extract class -Set 2

commit 9535176d12c06d76ddab77a949c065b884921561 Author: Alen John al283652@dal.ca Date: Wed Apr 5 14:59:04 2023 -0300

Rename variable - Set 1 -  done

commit a0ff0fbfcc72ceed1ffb8afc6131af59cb27c758 Author: Alen John al283652@dal.ca Date: Wed Apr 5 14:35:44 2023 -0300

Decompose conditional statement - Set 1