My First Refactor – Part 2

Continuing from my previous post, I’ve created a characterisation test for my Stock Analysis application. I captured the HTML output it produced and wrote a simple test which ensured that it always produced the same output. I used PHPUnit as the testing framework and used composer.js for dependency management.

Creating the Characterisation Test

Part of my initial code produced a raw HTML <table> with the results of the investment strategy. You can see what this looks like from this screenshot:

I restructured the code from being all in one index.php file to being in two files engine.php and index.php. In doing so I broke up the code into two partitions:

I moved all the logic which interacted with the database and which produced the <table> into the engine.php and kept the <form> and application title elements in the index.php.

This is good design because now the Application partition can focus on processing the data and the Main partition can focus on displaying the data to the user and interacting with the user. At a high level at least this application structure now follows the Single Responsibility Principle. Meaning that each part of the application should do one and only one thing. So now, my index.php looks like this:

<h1>Dollar Cost Average Calculator</h1>

<form method="get" action="index.php">
	<label>Euro per month: </label>
	<input name="euro_per_month" type="text"/> 
	<input type="submit" value="Dollar Cost Average"/>
</form>

<?php

include "return_on_investment/engine.php";

if (isset($_GET['euro_per_month'])) {
	$euro_per_month = $_GET['euro_per_month'];
}

echo calculateReturnOnInvestment();

?>

<h1>Dollar Cost Average Calculator</h1>

<label>Euro per month: </label>

</form>

<?php

include "return_on_investment/engine.php";

if (isset($_GET['euro_per_month'])) {

$euro_per_month = $_GET['euro_per_month'];

}

echo calculateReturnOnInvestment();

I hope you’ll agree that this looks much simpler than before and easier to read. This restructuring also enables me to capture the output of the new calculateReturnOnInvestment() method. This method returns the full HTML table element. So I can run it from the php interactive console and then copy and paste the output into a new file called golden_output.php. This file contains a method called getGoldenOutput() which returns a string with the “golden” HTML which I will use for comparison.

Finally I write the test. The test itself is a comparison of the golden output and what the calculateReturnOnInvestment() method currently produces. You can see the test here:

require_once 'engine.php';
require_once 'golden_output.php';

class DollarCostAveragerTests extends PHPUnit_Framework_TestCase
{
    public function test_Dollar_Cost_Averager_characteristics()
    {
		$this->assertEquals(getGoldenOutput(), 
                            calculateReturnOnInvestment());
    }
}

require_once 'engine.php';

require_once 'golden_output.php';

class DollarCostAveragerTests extends PHPUnit_Framework_TestCase

{

public function test_Dollar_Cost_Averager_characteristics()

{

$this->assertEquals(getGoldenOutput(),

calculateReturnOnInvestment());

}

I run my test using PHPUnit’s CLI interface:

matthewlynnsmbp:return_on_investment matthewoflynn$ phpunit tests/tests.php 
PHPUnit 3.7.31 by Sebastian Bergmann.

.

Time: 126 ms, Memory: 3.75Mb

OK (1 test, 1 assertion)

matthewlynnsmbp:return_on_investment matthewoflynn$ phpunit tests/tests.php

PHPUnit 3.7.31 by Sebastian Bergmann.

Time: 126 ms, Memory: 3.75Mb

OK (1 test, 1 assertion)

Running this test now instantly informs me if I change the current behaviour of the code. So I can focus entirely on performing the refactor without worrying that I’ve broken anything.

Refactoring the Application Partition

So far, to get my application under test I’ve just copied the hard to read blob of code into a single function. I need to break this function up so that the code is more understandable and easy to change. I turned to Martin Fowler’s book ” Refactoring – Improving the Design of Existing Code” to learn about the different mechanisms for refactoring.

Martin Fowler - Refactoring Improving the Design of Existing Code — Refactoring Improving the Design of Existing Code

Mr. Fowler provides a catalogue of refactoring techniques which I’ve tried to employ during my refactoring efforts. In this section, I highlight examples of how I used these techniques. To start, lets have a look at the calculateReturnOnInvestment() function:

function calculateDollarCostAverage() {

	/// ------------------------------------------
	$euro_per_month = 500;
	$cost_per_month = 3;
	$dividend_per_quarter = 0.005;
	$USA_DIVIDEND_WITHHOLDING_TAX = 0.15;
	$IRISH_INCOME_TAX_ON_US_DIVIDENDS = 0.41 - $USA_DIVIDEND_WITHHOLDING_TAX;
	$IRISH_TAX_PAYMENT_MONTH = 1;

	if (isset($_GET['euro_per_month'])) {
		$euro_per_month = $_GET['euro_per_month'];
	}
	/// ------------------------------------------

	$mysqli = new mysqli("127.0.0.1", "invest_data_user", "1234qwer", "investing_data");

	$result = $mysqli->query("SELECT * FROM import_test_2 " .
							 "WHERE date BETWEEN '2005-12-01' AND '2012-12-31' " .
							 "ORDER BY date ASC");

	$table_html = '<table border="1" cellpadding="5">';
	$table_html .= '<tr>';
	$table_html .= '<th>Date</th>';
	$table_html .= '<th>Price</th>';

	$table_html .= '<th>&nbsp;&nbsp;&nbsp;</th>';

	$table_html .= '<th>Cash to Invest</th>';
	$table_html .= '<th>Shares Bought</th>';
	$table_html .= '<th>Shares Owned</th>';
	$table_html .= '<th>Cash Invested</th>';
	$table_html .= '<th>Stock Worth</th>';
	$table_html .= '<th>Dividends Paid</th>';
	$table_html .= '<th>Gross Dividends YTD</th>';
	$table_html .= '<th>Cash Balance</th>';

	$table_html .= '<th>&nbsp;&nbsp;&nbsp;</th>';

	$table_html .= '<th>Return</th>';
	$table_html .= '</tr>';

	$current_month = 2;
	$day = 1;

	/// ------------------------------------------
	$euro_invested = 0;
	$cash_balance = 0;
	$shares_purchased = 0;
	$day_of_month_to_buy_on = 1;
	$gross_dividends_this_year = 0;
	/// ------------------------------------------

	while($row = $result->fetch_assoc())
	{
		$date = DateTime::createFromFormat('Y-m-d', $row['date']);
		$month = $date->format('n');

		if ($month == $current_month) {

			/// ------------------------------------------
			/// filter out 0 values
			if ($row['high'] == 0) {
				continue;
			}
			/// ------------------------------------------

			if ($day < $day_of_month_to_buy_on) {
				$day++;
				continue;
			}

			$table_html .= '<tr>';

			$table_html .= '<td>' . $row['date'] . '</td>';
			$table_html .= '<td>&euro;' . $row['high'] . '</td>';
			$table_html .= '<td> </td>';

			/// ------------------------------------------
			$euro_invested += $euro_per_month;
			$cash_balance += $euro_per_month;
			$cash_at_start_of_month = $cash_balance;
			$cash_balance -= $cost_per_month;
			$price_per_share = $row['high'];
			$num_shares_can_buy = intval($cash_balance / $price_per_share);
			$shares_purchased += $num_shares_can_buy;
			$stock_worth = $shares_purchased * $price_per_share;
			$cash_balance -= $num_shares_can_buy * $price_per_share;
			$dividend_this_month = ($current_month==3 or $current_month==6 or$current_month==9 or$current_month==12) ? 1 : 0;
			$dividends_paid = $shares_purchased * $price_per_share * $dividend_per_quarter * $dividend_this_month;
			$gross_dividends_this_year += $dividends_paid;
			$dividend_this_month -= $USA_DIVIDEND_WITHHOLDING_TAX * $dividends_paid; 
			$cash_balance += $dividends_paid;

			if ($current_month == $IRISH_TAX_PAYMENT_MONTH) {
				$tax_payable_to_revenue_on_us_dividends = $IRISH_INCOME_TAX_ON_US_DIVIDENDS * $gross_dividends_this_year;
				$cash_balance -= $tax_payable_to_revenue_on_us_dividends;

				// reset dividend aggregate 
				$gross_dividends_this_year = 0;
			}

			$percentage_return = intval(($cash_balance+$stock_worth) * 100 / $euro_invested) - 100;

			/// ------------------------------------------

			$table_html .= '<td>&euro;' . $cash_at_start_of_month . '</td>';
			$table_html .= '<td>x' . $num_shares_can_buy . '</td>';
			$table_html .= '<td>x' . $shares_purchased . '</td>';
			$table_html .= '<td>&euro;' . $euro_invested . '</td>';
			$table_html .= '<td>&euro;' . $stock_worth . '</td>';
			$table_html .= '<td>&euro;' . $dividends_paid . '</td>';
			$table_html .= '<td>&euro;' . $gross_dividends_this_year . '</td>';
			$table_html .= '<td>&euro;' . $cash_balance . '</td>';
			$table_html .= '<td> </td>';
			$table_html .= '<td>' . $percentage_return . '%</td>';

			$table_html .= '</tr>';

			/// ------------------------------------------

			/// ------------------------------------------

			$current_month++;
			if ($current_month == 13) {
				$current_month = 1;
			}
		}
	}

	$table_html .= '</table>';

	return $table_html;
}

100

101

102

103

104

105

106

107

108

109

110

111

112

113

114

115

116

117

118

119

120

121

122

123

124

125

126

127

128

129

130

131

132

133

134

function calculateDollarCostAverage() {

/// ------------------------------------------

$euro_per_month = 500;

$cost_per_month = 3;

$dividend_per_quarter = 0.005;

$USA_DIVIDEND_WITHHOLDING_TAX = 0.15;

$IRISH_INCOME_TAX_ON_US_DIVIDENDS = 0.41 - $USA_DIVIDEND_WITHHOLDING_TAX;

$IRISH_TAX_PAYMENT_MONTH = 1;

if (isset($_GET['euro_per_month'])) {

$euro_per_month = $_GET['euro_per_month'];

}

/// ------------------------------------------

$mysqli = new mysqli("127.0.0.1", "invest_data_user", "1234qwer", "investing_data");

$result = $mysqli->query("SELECT * FROM import_test_2 " .

"WHERE date BETWEEN '2005-12-01' AND '2012-12-31' " .

"ORDER BY date ASC");

$table_html = '<table border="1" cellpadding="5">';

$table_html .= '<tr>';

$table_html .= '<th>Date</th>';

$table_html .= '<th>Price</th>';

$table_html .= '<th>   </th>';

$table_html .= '<th>Cash to Invest</th>';

$table_html .= '<th>Shares Bought</th>';

$table_html .= '<th>Shares Owned</th>';

$table_html .= '<th>Cash Invested</th>';

$table_html .= '<th>Stock Worth</th>';

$table_html .= '<th>Dividends Paid</th>';

$table_html .= '<th>Gross Dividends YTD</th>';

$table_html .= '<th>Cash Balance</th>';

$table_html .= '<th>   </th>';

$table_html .= '<th>Return</th>';

$table_html .= '</tr>';

$current_month = 2;

$day = 1;

/// ------------------------------------------

$euro_invested = 0;

$cash_balance = 0;

$shares_purchased = 0;

$day_of_month_to_buy_on = 1;

$gross_dividends_this_year = 0;

/// ------------------------------------------

while($row = $result->fetch_assoc())

{

$date = DateTime::createFromFormat('Y-m-d', $row['date']);

$month = $date->format('n');

if ($month == $current_month) {

/// ------------------------------------------

/// filter out 0 values

if ($row['high'] == 0) {

continue;

}

/// ------------------------------------------

if ($day < $day_of_month_to_buy_on) {

$day++;

continue;

}

$table_html .= '<tr>';

$table_html .= '<td>' . $row['date'] . '</td>';

$table_html .= '<td>€' . $row['high'] . '</td>';

$table_html .= '<td> </td>';

/// ------------------------------------------

$euro_invested += $euro_per_month;

$cash_balance += $euro_per_month;

$cash_at_start_of_month = $cash_balance;

$cash_balance -= $cost_per_month;

$price_per_share = $row['high'];

$num_shares_can_buy = intval($cash_balance / $price_per_share);

$shares_purchased += $num_shares_can_buy;

$stock_worth = $shares_purchased * $price_per_share;

$cash_balance -= $num_shares_can_buy * $price_per_share;

$dividend_this_month = ($current_month==3 or $current_month==6 or$current_month==9 or$current_month==12) ? 1 : 0;

$dividends_paid = $shares_purchased * $price_per_share * $dividend_per_quarter * $dividend_this_month;

$gross_dividends_this_year += $dividends_paid;

$dividend_this_month -= $USA_DIVIDEND_WITHHOLDING_TAX * $dividends_paid;

$cash_balance += $dividends_paid;

if ($current_month == $IRISH_TAX_PAYMENT_MONTH) {

$tax_payable_to_revenue_on_us_dividends = $IRISH_INCOME_TAX_ON_US_DIVIDENDS * $gross_dividends_this_year;

$cash_balance -= $tax_payable_to_revenue_on_us_dividends;

// reset dividend aggregate

$gross_dividends_this_year = 0;

}

$percentage_return = intval(($cash_balance+$stock_worth) * 100 / $euro_invested) - 100;

/// ------------------------------------------

$table_html .= '<td>€' . $cash_at_start_of_month . '</td>';

$table_html .= '<td>x' . $num_shares_can_buy . '</td>';

$table_html .= '<td>x' . $shares_purchased . '</td>';

$table_html .= '<td>€' . $euro_invested . '</td>';

$table_html .= '<td>€' . $stock_worth . '</td>';

$table_html .= '<td>€' . $dividends_paid . '</td>';

$table_html .= '<td>€' . $gross_dividends_this_year . '</td>';

$table_html .= '<td>€' . $cash_balance . '</td>';

$table_html .= '<td> </td>';

$table_html .= '<td>' . $percentage_return . '%</td>';

$table_html .= '</tr>';

/// ------------------------------------------

$current_month++;

if ($current_month == 13) {

$current_month = 1;

}

$table_html .= '</table>';

return $table_html;

}

The first thing you might notice about this code is that it’s hard to follow. So how to improve this code? As I mentioned in a previous blog post, Robert C. Martin suggests that “classes hide in long functions”. You can see that the function has lots of temporary variables and various blocks of logic all using those variables. This sounds like those blocks could be methods for the class and the temporary variables could be members. So in this case changing this function into a class sounds like an ideal first step.

Refactoring Technique: Extract Class

To do this, I create a new class called ReturnOnInvestmentTablePrinter and move the logic into it. While it might seem that I’m just bloating the system with more code, creating this class now enables me to employ more refactoring techniques. I’m now able to split out this method into different code blocks.

Using this technique, I split up the code into header, rows and footer generation methods. Now the generateReturnOnInvestmentTable() method is bit easier to read:

class ReturnOnInvestmentTablePrinter {
	const DOLLAR_COST_PER_MONTH = 3;
	const PERCENT_USA_DIVIDEND_WITHHOLDING_TAX = 0.15;
	const PERCENT_IRISH_HIGHER_RATE_INCOME_TAX = 0.41;
	const PERCENT_DIVIDEND_PER_QUARTER = 0.005;

	private $current_month = 2;
	private $current_day = 1;

	private $euro_invested;
	private $cash_balance;

	private $strategy;

	function __construct($strategy) {
		$this->strategy = $strategy;
	}

	function generateReturnOnInvestmentTable() {
		return $this->getTableHeaderHTML()
			 . $this->getTableRowsHTML()
			 . $this->getTableFooterHTML();
	}

	private function getTableHeaderHTML() {
		$table_header_html = '<table border="1" cellpadding="5">'
						   . '<tr>'
						   . '<th>Date</th>'
						   . '<th>Price</th>'

						   . '<th>&nbsp;&nbsp;&nbsp;</th>'

						   . '<th>Cash to Invest</th>'
						   . '<th>Shares Bought</th>'
						   . '<th>Shares Owned</th>'
						   . '<th>Cash Invested</th>'
						   . '<th>Stock Worth</th>'
						   . '<th>Dividends Paid</th>'
						   . '<th>Gross Dividends YTD</th>'
						   . '<th>Cash Balance</th>'

						   . '<th>&nbsp;&nbsp;&nbsp;</th>'

						   . '<th>Return</th>'
						   . '</tr>';

		return $table_header_html;
	}

	private function getTableRowsHTML() {
		$table_rows_html = '';
		$result = $this->strategy->getStockPrices();

		/// ------------------------------------------
		$euro_per_month = $this->strategy->getEuroPerMonth();

		$percent_irish_income_tax_to_pay = 
			ReturnOnInvestmentTablePrinter::PERCENT_IRISH_HIGHER_RATE_INCOME_TAX 
			- ReturnOnInvestmentTablePrinter::PERCENT_USA_DIVIDEND_WITHHOLDING_TAX;
		$IRISH_TAX_PAYMENT_MONTH = 1;

		$shares_purchased = 0;
		$day_of_month_to_buy_on = 1;
		$gross_dividends_this_year = 0;
		/// ------------------------------------------

		while($row = $result->fetch_assoc())
		{
			$date = DateTime::createFromFormat('Y-m-d', $row['date']);
			$month = $date->format('n');

			if ($month == $this->current_month) {

				/// ------------------------------------------
				/// filter out 0 values
				if ($row['high'] == 0) {
					continue;
				}
				/// ------------------------------------------

				if ($this->current_day < $day_of_month_to_buy_on) {
					$this->current_day++;
					continue;
				}

				/// ------------------------------------------
				$this->euro_invested += $euro_per_month;
				$this->cash_balance += $euro_per_month;
				$cash_at_start_of_month = $this->cash_balance;
				$this->cash_balance -= ReturnOnInvestmentTablePrinter::DOLLAR_COST_PER_MONTH;
				$price_per_share = $row['high'];
				$num_shares_can_buy = intval($this->cash_balance / $price_per_share);
				$shares_purchased += $num_shares_can_buy;
				$stock_worth = $shares_purchased * $price_per_share;
				$this->cash_balance -= $num_shares_can_buy * $price_per_share;
				$dividend_this_month = ($this->current_month==3 or $this->current_month==6 or$this->current_month==9 or$this->current_month==12) ? 1 : 0;
				$dividends_paid = $shares_purchased 
								  * $price_per_share 
								  * ReturnOnInvestmentTablePrinter::PERCENT_DIVIDEND_PER_QUARTER 
								  * $dividend_this_month;
				$gross_dividends_this_year += $dividends_paid;
				$dividend_this_month -= ReturnOnInvestmentTablePrinter::PERCENT_USA_DIVIDEND_WITHHOLDING_TAX * $dividends_paid; 
				$this->cash_balance += $dividends_paid;

				if ($this->current_month == $IRISH_TAX_PAYMENT_MONTH) {
					$tax_payable_to_revenue_on_us_dividends = $percent_irish_income_tax_to_pay * $gross_dividends_this_year;
					$this->cash_balance -= $tax_payable_to_revenue_on_us_dividends;

					// reset dividend aggregate 
					$gross_dividends_this_year = 0;
				}

				$percentage_return = intval(($this->cash_balance+$stock_worth) * 100 / $this->euro_invested) - 100;

				/// ------------------------------------------

				$table_rows_html .= $this->getTableRowHTML($row['date'], 
							 $row['high'],
							 $cash_at_start_of_month,
							 $num_shares_can_buy,
							 $shares_purchased,
							 $stock_worth,
							 $dividends_paid,
							 $gross_dividends_this_year,
							 $percentage_return);

				/// ------------------------------------------

				/// ------------------------------------------

				$this->current_month++;
				if ($this->current_month == 13) {
					$this->current_month = 1;
				}
			}

		}

		return $table_rows_html;
	}	

	private function getTableRowHTML($date, 
									 $high,
									 $cash_at_start_of_month,
									 $num_shares_can_buy,
									 $shares_purchased,
									 $stock_worth,
									 $dividends_paid,
									 $gross_dividends_this_year,
									 $percentage_return ) 
	{
				$table_row_html = '<tr>'

							    . '<td>' . $date . '</td>'
							    . '<td>&euro;' . $high . '</td>'
							    . '<td> </td>'
							    . '<td>&euro;' . $cash_at_start_of_month . '</td>'
							    . '<td>x' . $num_shares_can_buy . '</td>'
							    . '<td>x' . $shares_purchased . '</td>'
							    . '<td>&euro;' . $this->euro_invested . '</td>'
							    . '<td>&euro;' . $stock_worth . '</td>'
							    . '<td>&euro;' . $dividends_paid . '</td>'
							    . '<td>&euro;' . $gross_dividends_this_year . '</td>'
							    . '<td>&euro;' . $this->cash_balance . '</td>'
							    . '<td> </td>'
							    . '<td>' . $percentage_return . '%</td>'

							    . '</tr>';

				return $table_row_html;
	}

	private function getTableFooterHTML() {
		return '</table>';
	}
};

100

101

102

103

104

105

106

107

108

109

110

111

112

113

114

115

116

117

118

119

120

121

122

123

124

125

126

127

128

129

130

131

132

133

134

135

136

137

138

139

140

141

142

143

144

145

146

147

148

149

150

151

152

153

154

155

156

157

158

159

160

161

162

163

164

165

166

167

168

169

170

171

172

173

174

175

176

class ReturnOnInvestmentTablePrinter {

const DOLLAR_COST_PER_MONTH = 3;

const PERCENT_USA_DIVIDEND_WITHHOLDING_TAX = 0.15;

const PERCENT_IRISH_HIGHER_RATE_INCOME_TAX = 0.41;

const PERCENT_DIVIDEND_PER_QUARTER = 0.005;

private $current_month = 2;

private $current_day = 1;

private $euro_invested;

private $cash_balance;

private $strategy;

function __construct($strategy) {

$this->strategy = $strategy;

}

function generateReturnOnInvestmentTable() {

return $this->getTableHeaderHTML()

. $this->getTableRowsHTML()

. $this->getTableFooterHTML();

}

private function getTableHeaderHTML() {

$table_header_html = '<table border="1" cellpadding="5">'

. '<tr>'

. '<th>Date</th>'

. '<th>Price</th>'

. '<th>   </th>'

. '<th>Cash to Invest</th>'

. '<th>Shares Bought</th>'

. '<th>Shares Owned</th>'

. '<th>Cash Invested</th>'

. '<th>Stock Worth</th>'

. '<th>Dividends Paid</th>'

. '<th>Gross Dividends YTD</th>'

. '<th>Cash Balance</th>'

. '<th>   </th>'

. '<th>Return</th>'

. '</tr>';

return $table_header_html;

}

private function getTableRowsHTML() {

$table_rows_html = '';

$result = $this->strategy->getStockPrices();

/// ------------------------------------------

$euro_per_month = $this->strategy->getEuroPerMonth();

$percent_irish_income_tax_to_pay =

ReturnOnInvestmentTablePrinter::PERCENT_IRISH_HIGHER_RATE_INCOME_TAX

- ReturnOnInvestmentTablePrinter::PERCENT_USA_DIVIDEND_WITHHOLDING_TAX;

$IRISH_TAX_PAYMENT_MONTH = 1;

$shares_purchased = 0;

$day_of_month_to_buy_on = 1;

$gross_dividends_this_year = 0;

/// ------------------------------------------

while($row = $result->fetch_assoc())

{

$date = DateTime::createFromFormat('Y-m-d', $row['date']);

$month = $date->format('n');

if ($month == $this->current_month) {

/// ------------------------------------------

/// filter out 0 values

if ($row['high'] == 0) {

continue;

}

/// ------------------------------------------

if ($this->current_day < $day_of_month_to_buy_on) {

$this->current_day++;

continue;

}

/// ------------------------------------------

$this->euro_invested += $euro_per_month;

$this->cash_balance += $euro_per_month;

$cash_at_start_of_month = $this->cash_balance;

$this->cash_balance -= ReturnOnInvestmentTablePrinter::DOLLAR_COST_PER_MONTH;

$price_per_share = $row['high'];

$num_shares_can_buy = intval($this->cash_balance / $price_per_share);

$shares_purchased += $num_shares_can_buy;

$stock_worth = $shares_purchased * $price_per_share;

$this->cash_balance -= $num_shares_can_buy * $price_per_share;

$dividend_this_month = ($this->current_month==3 or $this->current_month==6 or$this->current_month==9 or$this->current_month==12) ? 1 : 0;

$dividends_paid = $shares_purchased

* $price_per_share

* ReturnOnInvestmentTablePrinter::PERCENT_DIVIDEND_PER_QUARTER

* $dividend_this_month;

$gross_dividends_this_year += $dividends_paid;

$dividend_this_month -= ReturnOnInvestmentTablePrinter::PERCENT_USA_DIVIDEND_WITHHOLDING_TAX * $dividends_paid;

$this->cash_balance += $dividends_paid;

if ($this->current_month == $IRISH_TAX_PAYMENT_MONTH) {

$tax_payable_to_revenue_on_us_dividends = $percent_irish_income_tax_to_pay * $gross_dividends_this_year;

$this->cash_balance -= $tax_payable_to_revenue_on_us_dividends;

// reset dividend aggregate

$gross_dividends_this_year = 0;

}

$percentage_return = intval(($this->cash_balance+$stock_worth) * 100 / $this->euro_invested) - 100;

/// ------------------------------------------

$table_rows_html .= $this->getTableRowHTML($row['date'],

$row['high'],

$cash_at_start_of_month,

$num_shares_can_buy,

$shares_purchased,

$stock_worth,

$dividends_paid,

$gross_dividends_this_year,

$percentage_return);

/// ------------------------------------------

$this->current_month++;

if ($this->current_month == 13) {

$this->current_month = 1;

}

return $table_rows_html;

}

private function getTableRowHTML($date,

$high,

$cash_at_start_of_month,

$num_shares_can_buy,

$shares_purchased,

$stock_worth,

$dividends_paid,

$gross_dividends_this_year,

$percentage_return )

{

$table_row_html = '<tr>'

. '<td>' . $date . '</td>'

. '<td>€' . $high . '</td>'

. '<td> </td>'

. '<td>€' . $cash_at_start_of_month . '</td>'

. '<td>x' . $num_shares_can_buy . '</td>'

. '<td>x' . $shares_purchased . '</td>'

. '<td>€' . $this->euro_invested . '</td>'

. '<td>€' . $stock_worth . '</td>'

. '<td>€' . $dividends_paid . '</td>'

. '<td>€' . $gross_dividends_this_year . '</td>'

. '<td>€' . $this->cash_balance . '</td>'

. '<td> </td>'

. '<td>' . $percentage_return . '%</td>'

. '</tr>';

return $table_row_html;

}

private function getTableFooterHTML() {

return '</table>';

}

};

The getTableRowsHTML() method is still pretty long and complicated. There are lots of different code blocks and temporary variables within it. It looks like I should perform another Extract Class. This would separate the table printing code from the strategy simulation code. I create a new class called DollarCostAverageStrategy with a getInvestmentResults() method and move the code from getTableRowsHTML() into it.

DollarCostAverageStrategy::getInvestmentResults() will return an array with the the $monthly_data which the table printer class can print neatly into a <table> element. Now the getTableRowsHTML() method looks like this:

	private function getTableRowsHTML() {
		$table_rows_html = '';
		$monthly_data = $this->strategy->getInvestmentResults();

		foreach ($monthly_data as $month_data) {
			$table_rows_html .= $this->getTableRowHTML($month_data);
		}

		return $table_rows_html;
	}

private function getTableRowsHTML() {

$table_rows_html = '';

$monthly_data = $this->strategy->getInvestmentResults();

foreach ($monthly_data as $month_data) {

$table_rows_html .= $this->getTableRowHTML($month_data);

}

return $table_rows_html;

}

Refactoring Technique: Remove Temps

My current goal is to split up the big getInvestmentResults() method into smaller, more easily understood methods. However there are lots of temporary variables which are needed by all the different code blocks in this method. This is preventing me from breaking this long hard-to-understand method up. To get around this I need to remove these temporary variables. I have a number of options – I can promote a variables to be a member of the class so that it will be accessible from all methods. Alternatively I can replace a temporary variable with a query method and finally I can inline a temporary variable where it is used into the code.

I usually take one temporary variable at a time and perform the appropriate technique. Once complete, the code now looks like this:

class DollarCostAverageStrategy
{
	const DOLLAR_COST_PER_MONTH = 3;
	const PERCENT_USA_DIVIDEND_WITHHOLDING_TAX = 0.15;
	const PERCENT_IRISH_HIGHER_RATE_INCOME_TAX = 0.41;
	const PERCENT_DIVIDEND_PER_QUARTER = 0.005;
	const IRISH_TAX_PAYMENT_MONTH = 1;

	function __construct($stock_prices) {
		$this->stock_prices = $stock_prices;
	}

	function getInvestmentResults() {
		while($row_data = $this->stock_prices->fetch_assoc())
			$this->processStockPriceData($row_data);

		return $this->investment_results;
	}

	private function processStockPriceData($row_data) {
		if ($this->monthToInvestOn($row_data['date'])) {
			$this->current_share_price	= $row_data['high'];

			if ($this->dayToInvestOn())  {
				$this->calculateInvestmentResults($row_data['date']);
				$this->incrementCurrentMonth();
			}
		}
	}

	private function monthToInvestOn($date_str) {
		$date = DateTime::createFromFormat('Y-m-d', $date_str);
		$month = $date->format('n');

		return $month == $this->current_month;
	}

	private function dayToInvestOn() {
		/// filter out 0 values
		if ($this->current_share_price == 0)
			return false;

		if ($this->current_day < $this->day_of_month_to_buy_on) {
			$this->current_day++;
			return false;
		}

		return true;
	}

	private function calculateInvestmentResults($date) {
		$this->transferMoneyToBroker();
		$this->payBrokerCosts();
		$this->buyMostSharesPossibleWithCashBalance();
		$this->reinvestDividendsIfEarned();
		$this->payIrishTaxes();
		$this->storeThisMonthsInvestmentResults($date);		
	}

	private function transferMoneyToBroker() {
		$this->euro_invested += $this->euro_per_month;
		$this->cash_balance += $this->euro_per_month;
		$this->cash_at_start_of_month = $this->cash_balance;
	}

	private function payBrokerCosts() {
		$this->cash_balance -= DollarCostAverageStrategy::DOLLAR_COST_PER_MONTH;
	}

	private function buyMostSharesPossibleWithCashBalance() {
		$this->shares_purchased_in_current_month = intval($this->cash_balance / $this->current_share_price);
		$this->shares_purchased += $this->shares_purchased_in_current_month;
		$this->cash_balance -= $this->shares_purchased_in_current_month * $this->current_share_price;

	}

	private function reinvestDividendsIfEarned() {
		$this->gross_dividends_this_year += $this->calculateDividendPayment();
		$this->cash_balance += $this->calculateDividendPayment();
	}

	private function payIrishTaxes() {
		if ($this->current_month == DollarCostAverageStrategy::IRISH_TAX_PAYMENT_MONTH) {
			$tax_payable_to_revenue_on_us_dividends = $this->percentIrishTaxToPay() * $this->gross_dividends_this_year;
			$this->cash_balance -= $tax_payable_to_revenue_on_us_dividends;

			// reset dividend aggregate 
			$this->gross_dividends_this_year = 0;
		}
	}

	private function storeThisMonthsInvestmentResults($date) {
		$investment_result = array('date' => $date,
					 'share_price' => $this->current_share_price,
					 'cash_at_start_of_month' => $this->cash_at_start_of_month,
					 'num_shares_can_buy' => $this->shares_purchased_in_current_month,
					 'shares_purchased' => $this->shares_purchased,
					 'euro_invested' => $this->euro_invested,
					 'stock_worth' => $this->getStockWorth(),
					 'dividends_paid' => $this->calculateDividendPayment(),
					 'gross_dividends_this_year' => $this->gross_dividends_this_year,
					 'cash_balance' => $this->cash_balance,
					 'percentage_return' => intval(
					 							($this->cash_balance+$this->getStockWorth()) 
					 							* 100 / $this->euro_invested
					 							) - 100);

		array_push($this->investment_results, $investment_result);

	}

	private function calculateDividendPayment() {
		$dividend_this_month = ($this->current_month == 3 or 
								$this->current_month == 6 or
								$this->current_month == 9 or
								$this->current_month == 12) ? 1 : 0;

		return $this->shares_purchased 
				* $this->current_share_price 
				* DollarCostAverageStrategy::PERCENT_DIVIDEND_PER_QUARTER 
				* $dividend_this_month;
	}

	private function getStockWorth() {
		return $this->shares_purchased * $this->current_share_price;
	}

	private function percentIrishTaxToPay() {
		return DollarCostAverageStrategy::PERCENT_IRISH_HIGHER_RATE_INCOME_TAX 
			    - DollarCostAverageStrategy::PERCENT_USA_DIVIDEND_WITHHOLDING_TAX;
	}

	private function incrementCurrentMonth() {
		$this->current_month++;
		if ($this->current_month == 13) {
			$this->current_month = 1;
		}
	}

	private $current_month = 2;
	private $current_day = 1;

	private $euro_invested;
	private $cash_balance;
	private $euro_per_month = 500;

	private $shares_purchased = 0;
	private $day_of_month_to_buy_on = 1;
	private $gross_dividends_this_year = 0;

	private $current_share_price;

	private $cash_at_start_of_month;
	private $shares_purchased_in_current_month;

	private $investment_results = array();
}

100

101

102

103

104

105

106

107

108

109

110

111

112

113

114

115

116

117

118

119

120

121

122

123

124

125

126

127

128

129

130

131

132

133

134

135

136

137

138

139

140

141

142

143

144

145

146

147

148

149

150

151

152

153

154

155

156

157

class DollarCostAverageStrategy

{

const DOLLAR_COST_PER_MONTH = 3;

const PERCENT_USA_DIVIDEND_WITHHOLDING_TAX = 0.15;

const PERCENT_IRISH_HIGHER_RATE_INCOME_TAX = 0.41;

const PERCENT_DIVIDEND_PER_QUARTER = 0.005;

const IRISH_TAX_PAYMENT_MONTH = 1;

function __construct($stock_prices) {

$this->stock_prices = $stock_prices;

}

function getInvestmentResults() {

while($row_data = $this->stock_prices->fetch_assoc())

$this->processStockPriceData($row_data);

return $this->investment_results;

}

private function processStockPriceData($row_data) {

if ($this->monthToInvestOn($row_data['date'])) {

$this->current_share_price = $row_data['high'];

if ($this->dayToInvestOn()) {

$this->calculateInvestmentResults($row_data['date']);

$this->incrementCurrentMonth();

}

private function monthToInvestOn($date_str) {

$date = DateTime::createFromFormat('Y-m-d', $date_str);

$month = $date->format('n');

return $month == $this->current_month;

}

private function dayToInvestOn() {

/// filter out 0 values

if ($this->current_share_price == 0)

return false;

if ($this->current_day < $this->day_of_month_to_buy_on) {

$this->current_day++;

return false;

}

return true;

}

private function calculateInvestmentResults($date) {

$this->transferMoneyToBroker();

$this->payBrokerCosts();

$this->buyMostSharesPossibleWithCashBalance();

$this->reinvestDividendsIfEarned();

$this->payIrishTaxes();

$this->storeThisMonthsInvestmentResults($date);

}

private function transferMoneyToBroker() {

$this->euro_invested += $this->euro_per_month;

$this->cash_balance += $this->euro_per_month;

$this->cash_at_start_of_month = $this->cash_balance;

}

private function payBrokerCosts() {

$this->cash_balance -= DollarCostAverageStrategy::DOLLAR_COST_PER_MONTH;

}

private function buyMostSharesPossibleWithCashBalance() {

$this->shares_purchased_in_current_month = intval($this->cash_balance / $this->current_share_price);

$this->shares_purchased += $this->shares_purchased_in_current_month;

$this->cash_balance -= $this->shares_purchased_in_current_month * $this->current_share_price;

}

private function reinvestDividendsIfEarned() {

$this->gross_dividends_this_year += $this->calculateDividendPayment();

$this->cash_balance += $this->calculateDividendPayment();

}

private function payIrishTaxes() {

if ($this->current_month == DollarCostAverageStrategy::IRISH_TAX_PAYMENT_MONTH) {

$tax_payable_to_revenue_on_us_dividends = $this->percentIrishTaxToPay() * $this->gross_dividends_this_year;

$this->cash_balance -= $tax_payable_to_revenue_on_us_dividends;

// reset dividend aggregate

$this->gross_dividends_this_year = 0;

}

private function storeThisMonthsInvestmentResults($date) {

$investment_result = array('date' => $date,

'share_price' => $this->current_share_price,

'cash_at_start_of_month' => $this->cash_at_start_of_month,

'num_shares_can_buy' => $this->shares_purchased_in_current_month,

'shares_purchased' => $this->shares_purchased,

'euro_invested' => $this->euro_invested,

'stock_worth' => $this->getStockWorth(),

'dividends_paid' => $this->calculateDividendPayment(),

'gross_dividends_this_year' => $this->gross_dividends_this_year,

'cash_balance' => $this->cash_balance,

'percentage_return' => intval(

($this->cash_balance+$this->getStockWorth())

* 100 / $this->euro_invested

) - 100);

array_push($this->investment_results, $investment_result);

}

private function calculateDividendPayment() {

$dividend_this_month = ($this->current_month == 3 or

$this->current_month == 6 or

$this->current_month == 9 or

$this->current_month == 12) ? 1 : 0;

return $this->shares_purchased

* $this->current_share_price

* DollarCostAverageStrategy::PERCENT_DIVIDEND_PER_QUARTER

* $dividend_this_month;

}

private function getStockWorth() {

return $this->shares_purchased * $this->current_share_price;

}

private function percentIrishTaxToPay() {

return DollarCostAverageStrategy::PERCENT_IRISH_HIGHER_RATE_INCOME_TAX

- DollarCostAverageStrategy::PERCENT_USA_DIVIDEND_WITHHOLDING_TAX;

}

private function incrementCurrentMonth() {

$this->current_month++;

if ($this->current_month == 13) {

$this->current_month = 1;

}

private $current_month = 2;

private $current_day = 1;

private $euro_invested;

private $cash_balance;

private $euro_per_month = 500;

private $shares_purchased = 0;

private $day_of_month_to_buy_on = 1;

private $gross_dividends_this_year = 0;

private $current_share_price;

private $cash_at_start_of_month;

private $shares_purchased_in_current_month;

private $investment_results = array();

}

Notice how the readability of the code has improved, especially in the: getInvestmentResults(), processStockPriceData() and calculateInvestmentResults() methods.

Dependency Injection

You may have noticed that I now pass the stock price data array into the Strategy and pass the Strategy object into the Table Printer. PHP isn’t a strongly typed language so it might not be fully clear that I’m trying to ensure that the Table Printer doesn’t rely on the Dollar Cost Averaging Strategy, instead it relies on an object which can getInvestmentResults(). The table printer shouldn’t know how the investment results are generated – it just knows that it will be passed an object from which it can getInvestmentResults().

There is still some coupling between the table printer and the strategy – the printer still knows about the table headers and about all the type of results available in the $monthly_data array. This can be improved later but at least we’re moving towards a loosely coupled design.

Changing the Behaviour

Up until now all I’ve cared about is improving the design. The Characterisation Test helped me do this. Now that I want to change the implementation to do something different. I can’t use a catch-all test anymore because this will break as soon as I change how anything behaves.

To do this, I need to write some more finely grained tests so I can ensure that my existing behaviour will continue to work while I add more functionality.

My First Refactor – Part 2

Creating the Characterisation Test

Refactoring the Application Partition

Refactoring Technique: Extract Class

Refactoring Technique: Remove Temps

Dependency Injection

Changing the Behaviour

Leave a comment

Cancel reply