Clean up various code.

app/Factory/TransactionFactory.php

@@ -113,7 +113,7 @@ class TransactionFactory
         $destinationAccount = $this->findAccount($destinationType, $data['destination_id'], $data['destination_name']);
 
         if (null === $sourceAccount || null === $destinationAccount) {
-            throw new FireflyException('Could not determine source or destination account.');
+            throw new FireflyException('Could not determine source or destination account.', $data);
         }
 
         Log::debug(sprintf('Source type is "%s", destination type is "%s"', $sourceAccount->accountType->type, $destinationAccount->accountType->type));

app/Import/Converter/Amount.php

@@ -33,12 +33,12 @@ class Amount implements ConverterInterface
      * Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.
      * - Jamie Zawinski.
      *
-     *
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
+     * @SuppressWarnings(PHPMD.NPathComplexity)
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @param $value
      *
      * @return string
-     *
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function convert($value): string
     {
@@ -47,58 +47,42 @@ class Amount implements ConverterInterface
         }
         Log::debug(sprintf('Start with amount "%s"', $value));
         $original = $value;
-        $value           = (string)$value;
+        $value    = $this->stripAmount((string)$value);
-        $value           = $this->stripAmount($value);
-        $len             = \strlen($value);
-        $decimalPosition = $len - 3;
-        $altPosition     = $len - 2;
         $decimal  = null;
 
-        if (($len > 2 && '.' === $value[$decimalPosition]) || ($len > 2 && strpos($value, '.') > $decimalPosition)) {
+        if ($this->decimalIsDot($value)) {
             $decimal = '.';
             Log::debug(sprintf('Decimal character in "%s" seems to be a dot.', $value));
         }
-        if ($len > 2 && ',' === $value[$decimalPosition]) {
+
+        if ($this->decimalIsComma($value)) {
             $decimal = ',';
             Log::debug(sprintf('Decimal character in "%s" seems to be a comma.', $value));
         }
+
         // decimal character is null? find out if "0.1" or ".1" or "0,1" or ",1"
-        if ($len > 1 && ('.' === $value[$altPosition] || ',' === $value[$altPosition])) {
+        if ($this->alternativeDecimalSign($value)) {
-            $decimal = $value[$altPosition];
+            $decimal = $this->getAlternativeDecimalSign($value);
-            Log::debug(sprintf('Alternate search resulted in "%s" for decimal sign.', $decimal));
         }
 
         // decimal character still null? Search from the left for '.',',' or ' '.
         if (null === $decimal) {
-            Log::debug('Decimal is still NULL, probably number with >2 decimals. Search for a dot.');
+            $decimal = $this->findFromLeft($value);
-            $res = strrpos($value, '.');
-            if (!(false === $res)) {
-                // blandly assume this is the one.
-                Log::debug(sprintf('Searched from the left for "." in amount "%s", assume this is the decimal sign.', $value));
-                $decimal = '.';
-            }
-            unset($res);
         }
 
-        // if decimal is dot, replace all comma's and spaces with nothing. then parse as float (round to 4 pos)
+        // if decimal is dot, replace all comma's and spaces with nothing
-        if ('.' === $decimal) {
+        if (null !== $decimal) {
-            $search = [',', ' '];
+            $value = $this->replaceDecimal($decimal, $value);
-            $value  = str_replace($search, '', $value);
-            Log::debug(sprintf('Converted amount from "%s" to "%s".', $original, $value));
-        }
-        if (',' === $decimal) {
-            $search = ['.', ' '];
-            $value  = str_replace($search, '', $value);
-            $value  = str_replace(',', '.', $value);
             Log::debug(sprintf('Converted amount from "%s" to "%s".', $original, $value));
         }
+
         if (null === $decimal) {
             // replace all:
             $search = ['.', ' ', ','];
             $value  = str_replace($search, '', $value);
             Log::debug(sprintf('No decimal character found. Converted amount from "%s" to "%s".', $original, $value));
         }
-        if ($value{0} === '.') {
+        if ('.' === $value{0}) {
             $value = '0' . $value;
         }
 
@@ -114,18 +98,113 @@ class Amount implements ConverterInterface
         return $formatted;
     }
 
-    private function bcround($number, $scale = 0)
+    /**
+     * Check if the value has a dot or comma on an alternative place,
+     * catching strings like ",1" or ".5".
+     *
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function alternativeDecimalSign(string $value): bool
     {
-        $fix = "5";
+        $length      = \strlen($value);
-        for ($i = 0; $i < $scale; $i++) {
+        $altPosition = $length - 2;
-            $fix = "0$fix";
-        }
-        $number = bcadd($number, "0.$fix", $scale + 1);
 
-        return bcdiv($number, "1.0", $scale);
+        return $length > 1 && ('.' === $value[$altPosition] || ',' === $value[$altPosition]);
     }
 
     /**
+     * Helper function to see if the decimal separator is a comma.
+     *
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function decimalIsComma(string $value): bool
+    {
+        $length          = \strlen($value);
+        $decimalPosition = $length - 3;
+
+        return $length > 2 && ',' === $value[$decimalPosition];
+    }
+
+    /**
+     * Helper function to see if the decimal separator is a dot.
+     *
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function decimalIsDot(string $value): bool
+    {
+        $length          = \strlen($value);
+        $decimalPosition = $length - 3;
+
+        return ($length > 2 && '.' === $value[$decimalPosition]) || ($length > 2 && strpos($value, '.') > $decimalPosition);
+    }
+
+    /**
+     * Search from the left for decimal sign.
+     *
+     * @param string $value
+     *
+     * @return string
+     */
+    private function findFromLeft(string $value): ?string
+    {
+        $decimal = null;
+        Log::debug('Decimal is still NULL, probably number with >2 decimals. Search for a dot.');
+        $res = strrpos($value, '.');
+        if (!(false === $res)) {
+            // blandly assume this is the one.
+            Log::debug(sprintf('Searched from the left for "." in amount "%s", assume this is the decimal sign.', $value));
+            $decimal = '.';
+        }
+
+        return $decimal;
+    }
+
+    /**
+     * Returns the alternative decimal point used, such as a dot or a comma,
+     * from strings like ",1" or "0.5".
+     *
+     * @param string $value
+     *
+     * @return string
+     */
+    private function getAlternativeDecimalSign(string $value): string
+    {
+        $length      = \strlen($value);
+        $altPosition = $length - 2;
+
+        return $value[$altPosition];
+
+    }
+
+    /**
+     * @param string $decimal
+     * @param string $value
+     *
+     * @return string
+     */
+    private function replaceDecimal(string $decimal, string $value): string
+    {
+        $search = [',', ' ']; // default when decimal sign is a dot.
+        if (',' === $decimal) {
+            $search = ['.', ' '];
+        }
+        $value = str_replace($search, '', $value);
+
+        /** @noinspection CascadeStringReplacementInspection */
+        $value = str_replace(',', '.', $value);
+
+        return $value;
+    }
+
+    /**
+     * Strip amount from weird characters.
+     *
      * @param string $value
      *
      * @return string

app/Import/Converter/INGDebitCredit.php

@@ -34,7 +34,7 @@ class INGDebitCredit implements ConverterInterface
      *
      * @return int
      */
-    public function convert($value)
+    public function convert($value): int
     {
         Log::debug('Going to convert ing debit credit', ['value' => $value]);
 

app/Import/JobConfiguration/FakeJobConfiguration.php

@@ -40,20 +40,22 @@ class FakeJobConfiguration implements JobConfigurationInterface
 
     /**
      * Returns true when the initial configuration for this job is complete.
+     *  configuration array of job must have two values:
+     * 'artist' must be 'david bowie', case insensitive
+     * 'song' must be 'golden years', case insensitive.
+     * if stage is not "new", then album must be 'station to station'
      *
      * @return bool
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function configurationComplete(): bool
     {
-        // configuration array of job must have two values:
-        // 'artist' must be 'david bowie', case insensitive
-        // 'song' must be 'golden years', case insensitive.
-        // if stage is not "new", then album must be 'station to station'
         $config = $this->importJob->configuration;
-        if ($this->importJob->stage === 'new') {
+        if ('new' === $this->importJob->stage) {
-            return (isset($config['artist']) && 'david bowie' === strtolower($config['artist']))
+            return
-                   && (isset($config['song']) && 'golden years' === strtolower($config['song']))
+                isset($config['artist'], $config['song'], $config['apply-rules'])
-                   && isset($config['apply-rules']);
+                && 'david bowie' === strtolower($config['artist'])
+                && 'golden years' === strtolower($config['song']);
         }
 
         return isset($config['album']) && 'station to station' === strtolower($config['album']);
@@ -80,12 +82,12 @@ class FakeJobConfiguration implements JobConfigurationInterface
             $configuration['artist'] = $artist;
         }
 
-        if ($song === 'golden years') {
+        if ('golden years' === $song) {
             // store song
             $configuration['song'] = $song;
         }
 
-        if ($album === 'station to station') {
+        if ('station to station' === $album) {
             // store album
             $configuration['album'] = $album;
         }
@@ -96,8 +98,7 @@ class FakeJobConfiguration implements JobConfigurationInterface
         $this->repository->setConfiguration($this->importJob, $configuration);
         $messages = new MessageBag();
 
-        if (\count($configuration) !== 3) {
+        if (3 !== \count($configuration)) {
-
             $messages->add('some_key', 'Ignore this error: ' . \count($configuration));
         }
 
@@ -136,13 +137,13 @@ class FakeJobConfiguration implements JobConfigurationInterface
         if (null === $applyRules) {
             return 'import.fake.apply-rules';
         }
-        if (strtolower($artist) !== 'david bowie') {
+        if ('david bowie' !== strtolower($artist)) {
             return 'import.fake.enter-artist';
         }
-        if (strtolower($song) !== 'golden years') {
+        if ('golden years' !== strtolower($song)) {
             return 'import.fake.enter-song';
         }
-        if (strtolower($album) !== 'station to station' && $this->importJob->stage !== 'new') {
+        if ('new' !== $this->importJob->stage && 'station to station' !== strtolower($album)) {
             return 'import.fake.enter-album';
         }
 

app/Import/JobConfiguration/FileJobConfiguration.php

@@ -52,7 +52,7 @@ class FileJobConfiguration implements JobConfigurationInterface
      */
     public function configurationComplete(): bool
     {
-        return $this->importJob->stage === 'ready_to_run';
+        return 'ready_to_run' === $this->importJob->stage;
     }
 
     /**

app/Import/Mapper/AssetAccountIbans.php

@@ -49,7 +49,7 @@ class AssetAccountIbans implements MapperInterface
             if (\strlen($iban) > 0) {
                 $topList[$accountId] = $account->iban . ' (' . $account->name . ')';
             }
-            if (0 === \strlen($iban)) {
+            if ('' === $iban) {
                 $list[$accountId] = $account->name;
             }
         }

app/Import/Mapper/OpposingAccountIbans.php

@@ -55,10 +55,11 @@ class OpposingAccountIbans implements MapperInterface
             if (\strlen($iban) > 0) {
                 $topList[$accountId] = $account->iban . ' (' . $account->name . ')';
             }
-            if (0 === \strlen($iban)) {
+            if ('' === $iban) {
                 $list[$accountId] = $account->name;
             }
         }
+        /** @noinspection AdditionOperationOnArraysInspection */
         $list = $topList + $list;
         asort($list);
         $list = [0 => (string)trans('import.map_do_not_map')] + $list;

app/Import/Prerequisites/FakePrerequisites.php

@@ -58,7 +58,7 @@ class FakePrerequisites implements PrerequisitesInterface
             $apiKey = app('preferences')->getForUser($this->user, 'fake_api_key', null)->data;
         }
         $oldKey = (string)\request()->old('api_key');
-        if ($oldKey !== '') {
+        if ('' !== $oldKey) {
             $apiKey = \request()->old('api_key'); // @codeCoverageIgnore
         }
 
@@ -118,7 +118,7 @@ class FakePrerequisites implements PrerequisitesInterface
         if (null === $apiKey->data) {
             return false;
         }
-        if (\strlen((string)$apiKey->data) === 32) {
+        if (32 === \strlen((string)$apiKey->data)) {
             return true;
         }
 

app/Import/Routine/FakeRoutine.php

@@ -55,7 +55,7 @@ class FakeRoutine implements RoutineInterface
     public function run(): void
     {
         Log::debug(sprintf('Now in run() for fake routine with status: %s', $this->importJob->status));
-        if ($this->importJob->status !== 'ready_to_run') {
+        if ('ready_to_run' !== $this->importJob->status) {
             throw new FireflyException(sprintf('Fake job should have status "ready_to_run", not "%s"', $this->importJob->status)); // @codeCoverageIgnore
         }
 
@@ -95,7 +95,6 @@ class FakeRoutine implements RoutineInterface
     /**
      * @param ImportJob $importJob
      *
-     * @return
      */
     public function setImportJob(ImportJob $importJob): void
     {

app/Import/Routine/SpectreRoutine.php

@@ -66,7 +66,7 @@ class SpectreRoutine implements RoutineInterface
                     $handler->run();
 
                     // if count logins is zero, go to authenticate stage
-                    if ($handler->getCountLogins() === 0) {
+                    if (0 === $handler->getCountLogins()) {
                         $this->repository->setStage($this->importJob, 'do-authenticate');
                         $this->repository->setStatus($this->importJob, 'ready_to_run');
 

app/Import/Specifics/AbnAmroDescription.php

@@ -122,8 +122,6 @@ class AbnAmroDescription implements SpecificInterface
     /**
      * Parses the current description in SEPA format.
      *
-     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
-     *
      * @return bool true if the description is SEPA format, false otherwise
      */
     protected function parseSepaDescription(): bool
@@ -165,7 +163,7 @@ class AbnAmroDescription implements SpecificInterface
             // Set a new description for the current transaction. If none was given
             // set the description to type, name and reference
             $this->row[7] = $newDescription;
-            if (0 === \strlen($newDescription)) {
+            if ('' === $newDescription) {
                 $this->row[7] = sprintf('%s - %s (%s)', $type, $name, $reference);
             }
 
@@ -220,7 +218,7 @@ class AbnAmroDescription implements SpecificInterface
                 // Set a new description for the current transaction. If none was given
                 // set the description to type, name and reference
                 $this->row[7] = $newDescription;
-                if (0 === \strlen($newDescription)) {
+                if ('' === $newDescription) {
                     $this->row[7] = sprintf('%s - %s (%s)', $type, $name, $reference);
                 }
             }

app/Import/Specifics/PresidentsChoice.php

@@ -55,7 +55,7 @@ class PresidentsChoice implements SpecificInterface
         $row = array_values($row);
         // first, if column 2 is empty and 3 is not, do nothing.
         // if column 3 is empty and column 2 is not, move amount to column 3, *-1
-        if (isset($row[3]) && 0 === \strlen($row[3])) {
+        if (isset($row[3]) && '' === $row[3]) {
             $row[3] = bcmul($row[2], '-1');
         }
         if (isset($row[1])) {

app/Import/Specifics/RabobankDescription.php

@@ -60,7 +60,7 @@ class RabobankDescription implements SpecificInterface
         $oppositeName    = isset($row[6]) ? trim($row[6]) : '';
         $alternateName   = isset($row[10]) ? trim($row[10]) : '';
 
-        if (\strlen($oppositeAccount) < 1 && \strlen($oppositeName) < 1) {
+        if ('' === $oppositeAccount && '' === $oppositeName) {
             Log::debug(
                 sprintf(
                     'Rabobank specific: Opposite account and opposite name are' .
@@ -71,7 +71,7 @@ class RabobankDescription implements SpecificInterface
             $row[6]  = $alternateName;
             $row[10] = '';
         }
-        if (!(\strlen($oppositeAccount) < 1 && \strlen($oppositeName) < 1)) {
+        if (!('' === $oppositeAccount && '' === $oppositeName)) {
             Log::debug('Rabobank specific: either opposite account or name are filled.');
         }
 

app/Import/Storage/ImportArrayStorage.php

@@ -102,7 +102,7 @@ class ImportArrayStorage
 
         // run rules, if configured to.
         $config = $this->importJob->configuration;
-        if (isset($config['apply-rules']) && $config['apply-rules'] === true) {
+        if (isset($config['apply-rules']) && true === $config['apply-rules']) {
             $this->setStatus('applying_rules');
             $this->applyRules($collection);
             $this->setStatus('rules_applied');
@@ -118,7 +118,6 @@ class ImportArrayStorage
      *
      * @param Collection $collection
      *
-     * @throws FireflyException
      */
     private function applyRules(Collection $collection): void
     {
@@ -153,7 +152,7 @@ class ImportArrayStorage
         foreach ($array as $index => $transaction) {
             if (strtolower(TransactionType::TRANSFER) === $transaction['type']) {
                 $count++;
-                Log::debug(sprintf('Row #%d is a transfer, increase count to %d', ($index + 1), $count));
+                Log::debug(sprintf('Row #%d is a transfer, increase count to %d', $index + 1, $count));
             }
         }
         if (0 === $count) {
@@ -179,7 +178,7 @@ class ImportArrayStorage
     {
         unset($transaction['importHashV2']);
         $json = json_encode($transaction);
-        if ($json === false) {
+        if (false === $json) {
             throw new FireflyException('Could not encode import array. Please see the logs.', $transaction); // @codeCoverageIgnore
         }
         $hash = hash('sha256', $json, false);
@@ -231,7 +230,6 @@ class ImportArrayStorage
      * @param string $hash
      *
      * @return int|null
-     * @throws FireflyException
      */
     private function hashExists(string $hash): ?int
     {
@@ -253,7 +251,7 @@ class ImportArrayStorage
      */
     private function linkToTag(Collection $collection): void
     {
-        if ($collection->count() === 0) {
+        if (0 === $collection->count()) {
             return;
         }
         /** @var TagRepositoryInterface $repository */
@@ -321,7 +319,7 @@ class ImportArrayStorage
             [
                 'description' => $transaction['description'] ?? '',
                 'amount'      => $transaction['transactions'][0]['amount'] ?? 0,
-                'date'        => isset($transaction['date']) ? $transaction['date'] : '',
+                'date'        => $transaction['date'] ?? '',
             ]
         );
     }
@@ -365,8 +363,7 @@ class ImportArrayStorage
                 );
                 continue;
             }
-            if ($this->checkForTransfers) {
+            if ($this->checkForTransfers && $this->transferExists($transaction)) {
-                if ($this->transferExists($transaction)) {
                 $this->logDuplicateTransfer($transaction);
                 $this->repository->addErrorMessage(
                     $this->importJob, sprintf(
@@ -377,12 +374,11 @@ class ImportArrayStorage
                 );
                 continue;
             }
-            }
             $transaction['importHashV2'] = $hash;
             $toStore[]                   = $transaction;
         }
         $count = \count($toStore);
-        if ($count === 0) {
+        if (0 === $count) {
             Log::info('No transactions to store left!');
 
             return new Collection;
@@ -411,7 +407,7 @@ class ImportArrayStorage
             Log::debug(sprintf('Going to store entry %d of %d', $index + 1, $count));
             // convert the date to an object:
             $store['date']        = Carbon::createFromFormat('Y-m-d', $store['date']);
-            $store['description'] = $store['description'] === '' ? '(empty description)' : $store['description'];
+            $store['description'] = '' === $store['description'] ? '(empty description)' : $store['description'];
             // store the journal.
             try {
                 $journal = $this->journalRepos->store($store);
@@ -445,7 +441,7 @@ class ImportArrayStorage
             return false;
         }
         // how many hits do we need?
-        $requiredHits = count($transaction['transactions']) * 4;
+        $requiredHits = \count($transaction['transactions']) * 4;
         $totalHits    = 0;
         Log::debug(sprintf('Required hits for transfer comparison is %d', $requiredHits));
         Log::debug(sprintf('Array has %d transactions.', \count($transaction['transactions'])));
@@ -504,6 +500,7 @@ class ImportArrayStorage
                 // compare source and destination id's
                 $transferSourceIDs = [(int)$transfer->account_id, (int)$transfer->opposing_account_id];
                 sort($transferSourceIDs);
+                /** @noinspection DisconnectedForeachInstructionInspection */
                 Log::debug('Comparing current transaction source+dest IDs', $currentSourceIDs);
                 Log::debug('.. with current transfer source+dest IDs', $transferSourceIDs);
                 if ($currentSourceIDs === $transferSourceIDs) {
@@ -515,6 +512,7 @@ class ImportArrayStorage
                 // compare source and destination names
                 $transferSource = [(string)$transfer->account_name, (int)$transfer->opposing_account_name];
                 sort($transferSource);
+                /** @noinspection DisconnectedForeachInstructionInspection */
                 Log::debug('Comparing current transaction source+dest names', $currentSourceNames);
                 Log::debug('.. with current transfer source+dest names', $transferSource);
                 if ($currentSourceNames === $transferSource) {