updated all code with cleancode standards et al

- made sure every method has a return value (KNAB) - changed [0-9] regexes to use \d instead - strict comparission checks for debit/credit values - removed case insensitive regex checks where applicable - array_merge in loop replaced with ```call_user_func_array('array_merge', $values)``` (@see https://github.com/dseguy/clearPHP/blob/master/rules/no-array_merge-in-loop.md)
fruitl00p · Mar 10, 2017 · 380a6c0 · 380a6c0
1 parent 8ad20bc
commit 380a6c0
Show file tree

Hide file tree

Showing 15 changed files with 122 additions and 127 deletions.
diff --git a/src/Banking/Transaction.php b/src/Banking/Transaction.php
@@ -177,15 +177,15 @@ public function getTransactionCode()
      */
     public function isDebit()
     {
-        return $this->getDebitCredit() == self::DEBIT;
+        return $this->getDebitCredit() === self::DEBIT;
     }
 
     /**
      * @return bool
      */
     public function isCredit()
     {
-        return $this->getDebitCredit() == self::CREDIT;
+        return $this->getDebitCredit() === self::CREDIT;
     }
 
     /**

diff --git a/src/Parser/Banking/Mt940/Engine.php b/src/Parser/Banking/Mt940/Engine.php
@@ -170,7 +170,7 @@ public function parse()
     protected function parseStatementData()
     {
         $results = preg_split(
-                '/(^:20:|^-X{,3}$|\Z)/sm',
+                '/(^:20:|^-X{,3}$|\Z)/m',
                 $this->getRawData(),
                 -1,
                 PREG_SPLIT_NO_EMPTY
@@ -310,7 +310,7 @@ protected function parseStatementTimestamp()
     {
         trigger_error('Deprecated in favor of splitting the start and end timestamps for a statement. '.
                 'Please use parseStatementStartTimestamp($format) or parseStatementEndTimestamp($format) instead. '.
-                'setTimestamp is now parseStatementStartTimestamp', E_USER_DEPRECATED);
+                'parseStatementTimestamp is now parseStatementStartTimestamp', E_USER_DEPRECATED);
 
         return $this->parseStatementStartTimestamp();
     }
@@ -529,7 +529,7 @@ protected function sanitizeAccount($string)
 
         // crude IBAN to 'old' converter
         if (Mt940::$removeIBAN
-                && preg_match('#[A-Z]{2}[0-9]{2}[A-Z]{4}(.*)#', $string, $results)
+                && preg_match('#[A-Z]{2}[\d]{2}[A-Z]{4}(.*)#', $string, $results)
                 && !empty($results[1])
         ) {
             $string = $results[1];
@@ -543,7 +543,7 @@ protected function sanitizeAccount($string)
                 ),
                 '0'
         );
-        if ($account != '' && strlen($account) < 9 && strpos($account, 'P') === false) {
+        if ($account !== '' && strlen($account) < 9 && strpos($account, 'P') === false) {
             $account = 'P'.$account;
         }
 
@@ -595,7 +595,7 @@ protected function sanitizeDescription($string)
     protected function sanitizeDebitCredit($string)
     {
         $debitOrCredit = strtoupper(substr((string) $string, 0, 1));
-        if ($debitOrCredit != Transaction::DEBIT && $debitOrCredit != Transaction::CREDIT) {
+        if ($debitOrCredit !== Transaction::DEBIT && $debitOrCredit !== Transaction::CREDIT) {
             trigger_error('wrong value for debit/credit ('.$string.')', E_USER_ERROR);
             $debitOrCredit = '';
         }

diff --git a/src/Parser/Banking/Mt940/Engine/Ing.php b/src/Parser/Banking/Mt940/Engine/Ing.php
@@ -41,7 +41,7 @@ protected function parseTransactionAccount()
                 return $this->sanitizeAccount($account);
             }
         }
-        if (preg_match('#:86:([A-Z]{2}[0-9]{2}[A-Z]{4}[\d]+?) [A-Z]{6}[A-Z0-9]{0,4} #', $transactionData, $results)) {
+        if (preg_match('#:86:([A-Z]{2}[\d]{2}[A-Z]{4}[\d]+?) [A-Z]{6}[A-Z0-9]{0,4} #', $transactionData, $results)) {
             $account = trim($results[1]);
             if (!empty($account)) {
                 return $this->sanitizeAccount($account);

diff --git a/src/Parser/Banking/Mt940/Engine/Knab.php b/src/Parser/Banking/Mt940/Engine/Knab.php
@@ -76,6 +76,8 @@ protected function parseTransactionAccount()
         ) {
             return $results['account'];
         }
+
+        return '';
     }
 
     /**
@@ -99,11 +101,14 @@ protected function parseTransactionAccountName()
                 && !empty($results[1])
         ) {
             return trim($results[1]);
-        } elseif (preg_match('#/NAME/(.*?)\n?/(REMI|CSID)/#ms', $this->getCurrentTransactionData(), $results)
+        }
+        if (preg_match('#/NAME/(.*?)\n?/(REMI|CSID)/#ms', $this->getCurrentTransactionData(), $results)
             && !empty($results[1])
         ) {
             return trim($results[1]);
         }
+
+        return '';
     }
 
     /**
@@ -126,6 +131,9 @@ protected function parseTransactionDescription()
         }
 
         $name = $this->parseTransactionAccountName();
+        if ($name === '') {
+            return $description;
+        }
         $accountNameIsInDescription = strpos($description, $name);
         if ($accountNameIsInDescription !== false) {
             return trim(substr($description, 0, $accountNameIsInDescription-6));

diff --git a/src/Parser/Banking/Mt940/Engine/Rabo.php b/src/Parser/Banking/Mt940/Engine/Rabo.php
@@ -35,7 +35,7 @@ protected function parseTransactionAccount()
             return $this->sanitizeAccount($results[1]);
         }
 
-        if (preg_match('/^:61:.{26}(.{16})/im', $this->getCurrentTransactionData(), $results)
+        if (preg_match('/^:61:.{26}(.{16})/m', $this->getCurrentTransactionData(), $results)
                 && !empty($results[1])
         ) {
             return $this->sanitizeAccount($results[1]);
@@ -114,7 +114,7 @@ protected function parseTransactionValueTimestamp()
     protected function sanitizeAccount($string)
     {
         $account = parent::sanitizeAccount($string);
-        if (strlen($account) > 20 && strpos($account, '80000') == 0) {
+        if (strlen($account) > 20 && strpos($account, '80000') === 0) {
             $account = substr($account, 5);
         }
 

diff --git a/src/Parser/Banking/Mt940/Engine/Spk.php b/src/Parser/Banking/Mt940/Engine/Spk.php
@@ -105,12 +105,8 @@ protected function parseTransactionDebitCredit()
     protected function parseTransactionCancellation()
     {
         $results = [];
-        if (preg_match('/^:61:\d+(R)?[CD].?\d+/', $this->getCurrentTransactionData(), $results)
-            && !empty($results[1])
-        ) {
-            return true;
-        }
-        return false;
+        return preg_match('/^:61:\d+(R)?[CD].?\d+/', $this->getCurrentTransactionData(), $results)
+            && !empty($results[1]);
     }
 
     /**
@@ -121,7 +117,7 @@ protected function parseTransactionCancellation()
     protected function parseStatementData()
     {
         return preg_split(
-                '/(^:20:|^-X{,3}$|\Z)/sm',
+                '/(^:20:|^-X{,3}$|\Z)/m',
                 $this->getRawData(),
                 -1,
                 PREG_SPLIT_NO_EMPTY

diff --git a/src/Parser/Banking/Mt940/Engine/Triodos.php b/src/Parser/Banking/Mt940/Engine/Triodos.php
@@ -66,7 +66,7 @@ protected function parseTransactionAccount()
     {
         $parts = $this->getDescriptionParts();
         $account = $parts[0];
-        if (preg_match('#[A-Z]{2}[0-9]{2}[A-Z]{4}(.*)#', $parts[2], $results)) {
+        if (preg_match('#[A-Z]{2}[\d]{2}[A-Z]{4}(.*)#', $parts[2], $results)) {
             $account = $parts[2];
         } elseif (preg_match('#10(\d+)#', $parts[0], $results)) {
             $account = $results[1];
@@ -91,7 +91,7 @@ private function getTransactionAccountParts()
     {
         $parts = $this->getDescriptionParts();
         array_shift($parts); // remove BBAN / BIC code
-        if (preg_match('#[A-Z]{2}[0-9]{2}[A-Z]{4}(.*)#', $parts[1], $results)) {
+        if (preg_match('#[A-Z]{2}[\d]{2}[A-Z]{4}(.*)#', $parts[1], $results)) {
             array_shift($parts); // remove IBAN too
             array_shift($parts); // remove IBAN some more
         }
@@ -136,7 +136,7 @@ private function getDescriptionParts()
     protected function parseStatementData()
     {
         return preg_split(
-                '/(^:20:|^-X{,3}$|\Z)/sm',
+                '/(^:20:|^-X{,3}$|\Z)/m',
                 $this->getRawData(),
                 -1,
                 PREG_SPLIT_NO_EMPTY

diff --git a/test/Banking/StatementTest.php b/test/Banking/StatementTest.php
@@ -137,13 +137,13 @@ public function testJsonSerialization()
         $expected = '{"bank":"ABN","account":"62.90.64.393","transactions":[],'.
                 '"startPrice":16250,"endPrice":6250,"startTimestamp":123,"endTimestamp":0,"number":"2665487AAF"}';
         $params = [
-                'bank' => 'ABN',
-                'account' => '62.90.64.393',
-                'transactions' => [],
-                'startPrice' => 16250,
-                'endPrice' => 6250,
-                'startTimestamp' => 123,
-                'number' => '2665487AAF',
+            'bank' => 'ABN',
+            'account' => '62.90.64.393',
+            'transactions' => [],
+            'startPrice' => 16250,
+            'endPrice' => 6250,
+            'startTimestamp' => 123,
+            'number' => '2665487AAF',
         ];
         $statement = new Statement();
         foreach ($params as $key => $value) {
@@ -159,39 +159,39 @@ public function testJsonSerialization()
     public function testJsonSerializationWithTransactions()
     {
         $expected = '{"bank":"ABN","account":"62.90.64.393","transactions":[{"account":"123123","accountName":'.
-                '"Kingsquare BV","price":110,"debitcredit":"D","description":"test","valueTimestamp":1231,"entryTimestamp"'.
-                ':1234,"transactionCode":"13G"},{"account":"123123","accountName":"Kingsquare BV","price":110,"debitcredit"'.
-                ':"D","description":"test","valueTimestamp":1231,"entryTimestamp":1234,"transactionCode":"13G"}],'.
-                '"startPrice":16250,"endPrice":6250,"number":"2665487AAF"}';
+            '"Kingsquare BV","price":110,"debitcredit":"D","description":"test","valueTimestamp":1231,"entryTimestamp"'.
+            ':1234,"transactionCode":"13G"},{"account":"123123","accountName":"Kingsquare BV","price":110,"debitcredit"'.
+            ':"D","description":"test","valueTimestamp":1231,"entryTimestamp":1234,"transactionCode":"13G"}],'.
+            '"startPrice":16250,"endPrice":6250,"number":"2665487AAF"}';
         $params = [
-                'bank' => 'ABN',
-                'account' => '62.90.64.393',
-                'transactions' => [
-                        [
-                                'account' => '123123',
-                                'accountName' => 'Kingsquare BV',
-                                'price' => 110.0,
-                                'debitcredit' => Transaction::DEBIT,
-                                'description' => 'test',
-                                'valueTimestamp' => 1231,
-                                'entryTimestamp' => 1234,
-                                'transactionCode' => '13G',
-                        ],
-                        [
-                                'account' => '123123',
-                                'accountName' => 'Kingsquare BV',
-                                'price' => 110.0,
-                                'debitcredit' => Transaction::DEBIT,
-                                'description' => 'test',
-                                'valueTimestamp' => 1231,
-                                'entryTimestamp' => 1234,
-                                'transactionCode' => '13G',
-                        ],
+            'bank' => 'ABN',
+            'account' => '62.90.64.393',
+            'transactions' => [
+                [
+                    'account' => '123123',
+                    'accountName' => 'Kingsquare BV',
+                    'price' => 110.0,
+                    'debitcredit' => Transaction::DEBIT,
+                    'description' => 'test',
+                    'valueTimestamp' => 1231,
+                    'entryTimestamp' => 1234,
+                    'transactionCode' => '13G',
                 ],
-                'startPrice' => 16250,
-                'endPrice' => 6250,
-                'timestamp' => 123,
-                'number' => '2665487AAF',
+                [
+                    'account' => '123123',
+                    'accountName' => 'Kingsquare BV',
+                    'price' => 110.0,
+                    'debitcredit' => Transaction::DEBIT,
+                    'description' => 'test',
+                    'valueTimestamp' => 1231,
+                    'entryTimestamp' => 1234,
+                    'transactionCode' => '13G',
+                ],
+            ],
+            'startPrice' => 16250,
+            'endPrice' => 6250,
+            'timestamp' => 123,
+            'number' => '2665487AAF',
         ];
         $statement = new Statement();
         foreach ($params as $key => $value) {

diff --git a/test/Parser/Banking/Mt940/Engine/Abn/ParseTest.php b/test/Parser/Banking/Mt940/Engine/Abn/ParseTest.php
@@ -12,17 +12,14 @@ class ParseTest extends \PHPUnit_Framework_TestCase
     /**
      * @var Abn
      */
-    private $engine = null;
+    private $engine;
 
     protected function setUp()
     {
         $this->engine = new Abn();
         $this->engine->loadString(file_get_contents(__DIR__.'/sample'));
     }
 
-    /**
-     *
-     */
     public function testParseStatementBank()
     {
         $method = new \ReflectionMethod($this->engine, 'parseStatementBank');
@@ -34,7 +31,7 @@ public function testParsesAllFoundStatements()
     {
         $statements = $this->engine->parse();
 
-        $this->assertEquals(4, count($statements));
+        $this->assertCount(4, $statements);
         $first = $statements[0];
         $last = end($statements);
 

diff --git a/test/Parser/Banking/Mt940/Engine/Ing/ParseTest.php b/test/Parser/Banking/Mt940/Engine/Ing/ParseTest.php
@@ -12,17 +12,14 @@ class ParseTest extends \PHPUnit_Framework_TestCase
     /**
      * @var Ing
      */
-    private $engine = null;
+    private $engine;
 
     protected function setUp()
     {
         $this->engine = new Ing();
         $this->engine->loadString(file_get_contents(__DIR__.'/sample'));
     }
 
-    /**
-     *
-     */
     public function testParseStatementBank()
     {
         $method = new \ReflectionMethod($this->engine, 'parseStatementBank');
@@ -34,7 +31,7 @@ public function testParsesAllFoundStatements()
     {
         $statements = $this->engine->parse();
 
-        $this->assertEquals(1, count($statements));
+        $this->assertCount(1, $statements);
         $first = $statements[0];
 
         $this->assertEquals('22-07-2010', $first->getStartTimestamp('d-m-Y'));

diff --git a/test/Parser/Banking/Mt940/Engine/Knab/ParseTest.php b/test/Parser/Banking/Mt940/Engine/Knab/ParseTest.php
@@ -12,7 +12,7 @@ class ParseTest extends \PHPUnit_Framework_TestCase
     /**
      * @var Knab
      */
-    private $engine = null;
+    private $engine;
 
     protected function setUp()
     {
@@ -31,7 +31,7 @@ public function testParsesAllFoundStatements()
     {
         $statements = $this->engine->parse();
 
-        $this->assertEquals(1, count($statements));
+        $this->assertCount(1, $statements);
         $this->assertEquals('03-12-2015', $statements[0]->getStartTimestamp('d-m-Y'));
         $this->assertEquals('03-12-2015', $statements[0]->getStartTimestamp('d-m-Y'));
     }

diff --git a/test/Parser/Banking/Mt940/Engine/Rabo/ParseTest.php b/test/Parser/Banking/Mt940/Engine/Rabo/ParseTest.php
@@ -12,7 +12,7 @@ class ParseTest extends \PHPUnit_Framework_TestCase
     /**
      * @var Rabo
      */
-    private $engine = null;
+    private $engine;
 
     protected function setUp()
     {
@@ -31,7 +31,7 @@ public function testParsesAllFoundStatements()
     {
         $statements = $this->engine->parse();
 
-        $this->assertEquals(39, count($statements));
+        $this->assertCount(39, $statements);
         $first = $statements[0];
         $last = end($statements);
         $this->assertEquals('06-01-2003', $first->getStartTimestamp('d-m-Y'));