Various code quality improvements.

This commit is contained in:
James Cole 2020-08-01 17:51:52 +02:00
parent ae7cd76d87
commit 1169c5c5e2
No known key found for this signature in database
GPG Key ID: B5669F9493CDE38D
7 changed files with 30 additions and 28 deletions

View File

@ -8,6 +8,7 @@ parameters:
ignoreErrors: ignoreErrors:
- '#is not allowed to extend#' - '#is not allowed to extend#'
- '#is neither abstract nor final#' - '#is neither abstract nor final#'
- '#Control structures using switch should not be used\.#'
paths: paths:
- ../app - ../app
- ../database - ../database

View File

@ -101,7 +101,7 @@ class OtherCurrenciesCorrections extends Command
if (array_key_exists($accountId, $this->accountCurrencies) && 0 === $this->accountCurrencies[$accountId]) { if (array_key_exists($accountId, $this->accountCurrencies) && 0 === $this->accountCurrencies[$accountId]) {
return null; // @codeCoverageIgnore return null; // @codeCoverageIgnore
} }
if (isset($this->accountCurrencies[$accountId]) && $this->accountCurrencies[$accountId] instanceof TransactionCurrency) { if (array_key_exists($accountId, $this->accountCurrencies) && $this->accountCurrencies[$accountId] instanceof TransactionCurrency) {
return $this->accountCurrencies[$accountId]; // @codeCoverageIgnore return $this->accountCurrencies[$accountId]; // @codeCoverageIgnore
} }
$currency = $this->accountRepos->getAccountCurrency($account); $currency = $this->accountRepos->getAccountCurrency($account);

View File

@ -335,10 +335,10 @@ class TransferCurrenciesCorrections extends Command
private function getCurrency(Account $account): ?TransactionCurrency private function getCurrency(Account $account): ?TransactionCurrency
{ {
$accountId = $account->id; $accountId = $account->id;
if (isset($this->accountCurrencies[$accountId]) && 0 === $this->accountCurrencies[$accountId]) { if (array_key_exists($accountId, $this->accountCurrencies) && 0 === $this->accountCurrencies[$accountId]) {
return null; // @codeCoverageIgnore return null; // @codeCoverageIgnore
} }
if (isset($this->accountCurrencies[$accountId]) && $this->accountCurrencies[$accountId] instanceof TransactionCurrency) { if (array_key_exists($accountId, $this->accountCurrencies) && $this->accountCurrencies[$accountId] instanceof TransactionCurrency) {
return $this->accountCurrencies[$accountId]; // @codeCoverageIgnore return $this->accountCurrencies[$accountId]; // @codeCoverageIgnore
} }
$currency = $this->accountRepos->getAccountCurrency($account); $currency = $this->accountRepos->getAccountCurrency($account);

View File

@ -59,15 +59,15 @@ class BillFactory
/** /**
* @param array $data * @param array $data
* *
* @throws FireflyException
* @return Bill|null * @return Bill|null
* @throws FireflyException
*/ */
public function create(array $data): ?Bill public function create(array $data): ?Bill
{ {
/** @var TransactionCurrencyFactory $factory */ /** @var TransactionCurrencyFactory $factory */
$factory = app(TransactionCurrencyFactory::class); $factory = app(TransactionCurrencyFactory::class);
/** @var TransactionCurrency $currency */ /** @var TransactionCurrency $currency */
$currency = $factory->find((int) ($data['currency_id'] ?? null), (string) ($data['currency_code'] ?? null)); $currency = $factory->find((int)($data['currency_id'] ?? null), (string)($data['currency_code'] ?? null));
if (null === $currency) { if (null === $currency) {
$currency = app('amount')->getDefaultCurrencyByUser($this->user); $currency = app('amount')->getDefaultCurrencyByUser($this->user);
@ -95,8 +95,8 @@ class BillFactory
throw new FireflyException('400000: Could not store bill.'); throw new FireflyException('400000: Could not store bill.');
} }
if (isset($data['notes'])) { if (array_key_exists('notes', $data)) {
$this->updateNote($bill, $data['notes']); $this->updateNote($bill, (string)$data['notes']);
} }
$objectGroupTitle = $data['object_group'] ?? ''; $objectGroupTitle = $data['object_group'] ?? '';
@ -108,7 +108,7 @@ class BillFactory
} }
} }
// try also with ID: // try also with ID:
$objectGroupId = (int) ($data['object_group_id'] ?? 0); $objectGroupId = (int)($data['object_group_id'] ?? 0);
if (0 !== $objectGroupId) { if (0 !== $objectGroupId) {
$objectGroup = $this->findObjectGroupById($objectGroupId); $objectGroup = $this->findObjectGroupById($objectGroupId);
if (null !== $objectGroup) { if (null !== $objectGroup) {
@ -128,8 +128,8 @@ class BillFactory
*/ */
public function find(?int $billId, ?string $billName): ?Bill public function find(?int $billId, ?string $billName): ?Bill
{ {
$billId = (int) $billId; $billId = (int)$billId;
$billName = (string) $billName; $billName = (string)$billName;
$bill = null; $bill = null;
// first find by ID: // first find by ID:
if ($billId > 0) { if ($billId > 0) {

View File

@ -57,8 +57,8 @@ class TransactionJournalFactory
use JournalServiceTrait; use JournalServiceTrait;
private AccountRepositoryInterface $accountRepository; private AccountRepositoryInterface $accountRepository;
private AccountValidator $accountValidator; private AccountValidator $accountValidator;
private BillRepositoryInterface $billRepository; private BillRepositoryInterface $billRepository;
/** @var CurrencyRepositoryInterface */ /** @var CurrencyRepositoryInterface */
private $currencyRepository; private $currencyRepository;
/** @var bool */ /** @var bool */
@ -87,7 +87,7 @@ class TransactionJournalFactory
{ {
$this->errorOnHash = false; $this->errorOnHash = false;
// TODO move valid meta fields to config. // TODO move valid meta fields to config.
$this->fields = [ $this->fields = [
// sepa // sepa
'sepa_cc', 'sepa_ct_op', 'sepa_ct_id', 'sepa_cc', 'sepa_ct_op', 'sepa_ct_id',
'sepa_db', 'sepa_country', 'sepa_ep', 'sepa_db', 'sepa_country', 'sepa_ep',
@ -128,9 +128,9 @@ class TransactionJournalFactory
* *
* @param array $data * @param array $data
* *
* @throws DuplicateTransactionException
* @throws FireflyException
* @return Collection * @return Collection
* @throws FireflyException
* @throws DuplicateTransactionException
*/ */
public function create(array $data): Collection public function create(array $data): Collection
{ {
@ -246,9 +246,9 @@ class TransactionJournalFactory
/** /**
* @param NullArrayObject $row * @param NullArrayObject $row
* *
* @throws FireflyException
* @throws DuplicateTransactionException
* @return TransactionJournal|null * @return TransactionJournal|null
* @throws DuplicateTransactionException
* @throws FireflyException
*/ */
private function createJournal(NullArrayObject $row): ?TransactionJournal private function createJournal(NullArrayObject $row): ?TransactionJournal
{ {
@ -306,10 +306,10 @@ class TransactionJournalFactory
Log::debug('Now calling getAccount for the destination.'); Log::debug('Now calling getAccount for the destination.');
$destinationAccount = $this->getAccount($type->type, 'destination', $destInfo); $destinationAccount = $this->getAccount($type->type, 'destination', $destInfo);
Log::debug('Done with getAccount(2x)'); Log::debug('Done with getAccount(2x)');
$currency = $this->getCurrencyByAccount($type->type, $currency, $sourceAccount, $destinationAccount); $currency = $this->getCurrencyByAccount($type->type, $currency, $sourceAccount, $destinationAccount);
$foreignCurrency = $this->compareCurrencies($currency, $foreignCurrency); $foreignCurrency = $this->compareCurrencies($currency, $foreignCurrency);
$foreignCurrency = $this->getForeignByAccount($type->type, $foreignCurrency, $destinationAccount); $foreignCurrency = $this->getForeignByAccount($type->type, $foreignCurrency, $destinationAccount);
$description = $this->getDescription($description); $description = $this->getDescription($description);
/** Create a basic journal. */ /** Create a basic journal. */
$journal = TransactionJournal::create( $journal = TransactionJournal::create(
@ -596,8 +596,8 @@ class TransactionJournalFactory
$this->accountValidator->setTransactionType($transactionType); $this->accountValidator->setTransactionType($transactionType);
// validate source account. // validate source account.
$sourceId = isset($data['source_id']) ? (int) $data['source_id'] : null; $sourceId = array_key_exists('source_id', $data) ? (int) $data['source_id'] : null;
$sourceName = isset($data['source_name']) ? (string) $data['source_name'] : null; $sourceName = array_key_exists('source_name', $data) ? (string) $data['source_name'] : null;
$validSource = $this->accountValidator->validateSource($sourceId, $sourceName, null); $validSource = $this->accountValidator->validateSource($sourceId, $sourceName, null);
// do something with result: // do something with result:
@ -606,8 +606,8 @@ class TransactionJournalFactory
} }
Log::debug('Source seems valid.'); Log::debug('Source seems valid.');
// validate destination account // validate destination account
$destinationId = isset($data['destination_id']) ? (int) $data['destination_id'] : null; $destinationId = array_key_exists('destination_id', $data) ? (int) $data['destination_id'] : null;
$destinationName = isset($data['destination_name']) ? (string) $data['destination_name'] : null; $destinationName = array_key_exists('destination_name', $data) ? (string) $data['destination_name'] : null;
$validDestination = $this->accountValidator->validateDestination($destinationId, $destinationName, null); $validDestination = $this->accountValidator->validateDestination($destinationId, $destinationName, null);
// do something with result: // do something with result:
if (false === $validDestination) { if (false === $validDestination) {

View File

@ -1,11 +1,11 @@
<?php <?php
declare(strict_types=1);
/** @var \Illuminate\Database\Eloquent\Factory $factory */ /** @var \Illuminate\Database\Eloquent\Factory $factory */
use Faker\Generator as Faker; use Faker\Generator as Faker;
use FireflyIII\Model; use FireflyIII\User;
$factory->define(Model::class, function (Faker $faker) { $factory->define(User::class, function (Faker $faker) {
return [ return [
// //
]; ];

View File

@ -1198,4 +1198,5 @@ try {
); );
} catch (DuplicateBreadcrumbException $e) { } catch (DuplicateBreadcrumbException $e) {
// @ignoreException
} }