Completed
Push — master ( 6c2cde...d72bee )
by Frederik
01:37
created

Record   B

Complexity

Total Complexity 40

Size/Duplication

Total Lines 186
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 14

Test Coverage

Coverage 78.56%

Importance

Changes 2
Bugs 0 Features 0
Metric Value
wmc 40
c 2
b 0
f 0
lcom 1
cbo 14
dl 0
loc 186
ccs 77
cts 98
cp 0.7856
rs 8.2608

3 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 5 1
F addEntries() 0 119 32
C addBalances() 0 35 7

How to fix   Complexity   

Complex Class

Complex classes like Record often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use Record, and based on these observations, apply Extract Interface, too.

1
<?php
2
3
namespace Genkgo\Camt\Decoder;
4
5
use Genkgo\Camt\DTO;
6
use Genkgo\Camt\Util\StringToUnits;
7
use Money\Money;
8
use Money\Currency;
9
use \SimpleXMLElement;
10
11
class Record
12
{
13
    /**
14
     * @var Entry
15
     */
16
    private $entryDecoder;
17
    /**
18
     * @var DateDecoderInterface
19
     */
20
    private $dateDecoder;
21
22
    /**
23
     * Record constructor.
24
     * @param Entry $entryDecoder
25
     * @param DateDecoderInterface $dateDecoder
26
     */
27 31
    public function __construct(Entry $entryDecoder, DateDecoderInterface $dateDecoder)
28
    {
29 31
        $this->entryDecoder = $entryDecoder;
30 31
        $this->dateDecoder = $dateDecoder;
31 31
    }
32
33
    /**
34
     * @param DTO\Record       $record
35
     * @param SimpleXMLElement $xmlRecord
36
     */
37 19
    public function addBalances(DTO\Record $record, SimpleXMLElement $xmlRecord)
38
    {
39 19
        $xmlBalances = $xmlRecord->Bal;
0 ignored issues
show
Bug introduced by
The property Bal does not seem to exist in SimpleXMLElement.

An attempt at access to an undefined property has been detected. This may either be a typographical error or the property has been renamed but there are still references to its old name.

If you really want to allow access to undefined properties, you can define magic methods to allow access. See the php core documentation on Overloading.

Loading history...
40 19
        foreach ($xmlBalances as $xmlBalance) {
41 14
            $amount = StringToUnits::convert((string) $xmlBalance->Amt);
42 14
            $currency = (string)$xmlBalance->Amt['Ccy'];
43 14
            $date = (string)$xmlBalance->Dt->Dt;
44
45 14
            if ((string) $xmlBalance->CdtDbtInd === 'DBIT') {
46 9
                $amount = $amount * -1;
47
            }
48
49 14
            if (isset($xmlBalance->Tp) && isset($xmlBalance->Tp->CdOrPrtry)) {
50 14
                $code = (string) $xmlBalance->Tp->CdOrPrtry->Cd;
51
52 14
                if (in_array($code, ['OPBD', 'PRCD'])) {
53 11
                    $record->addBalance(DTO\Balance::opening(
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Genkgo\Camt\DTO\Record as the method addBalance() does only exist in the following sub-classes of Genkgo\Camt\DTO\Record: Genkgo\Camt\Camt052\DTO\Report, Genkgo\Camt\Camt053\DTO\Statement. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
54 11
                        new Money(
55 11
                            $amount,
56 11
                            new Currency($currency)
57
                        ),
58 11
                        $this->dateDecoder->decode($date)
59
                    ));
60 13
                } elseif ($code === 'CLBD') {
61 10
                    $record->addBalance(DTO\Balance::closing(
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Genkgo\Camt\DTO\Record as the method addBalance() does only exist in the following sub-classes of Genkgo\Camt\DTO\Record: Genkgo\Camt\Camt052\DTO\Report, Genkgo\Camt\Camt053\DTO\Statement. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
62 10
                        new Money(
63 10
                            $amount,
64 10
                            new Currency($currency)
65
                        ),
66 14
                        $this->dateDecoder->decode($date)
67
                    ));
68
                }
69
            }
70
        }
71 19
    }
72
73
    /**
74
     * @param DTO\Record       $record
75
     * @param SimpleXMLElement $xmlRecord
76
     */
77 22
    public function addEntries(DTO\Record $record, SimpleXMLElement $xmlRecord)
78
    {
79 22
        $index = 0;
80 22
        $xmlEntries = $xmlRecord->Ntry;
0 ignored issues
show
Bug introduced by
The property Ntry does not seem to exist in SimpleXMLElement.

An attempt at access to an undefined property has been detected. This may either be a typographical error or the property has been renamed but there are still references to its old name.

If you really want to allow access to undefined properties, you can define magic methods to allow access. See the php core documentation on Overloading.

Loading history...
81 22
        foreach ($xmlEntries as $xmlEntry) {
82 21
            $amount      = StringToUnits::convert((string) $xmlEntry->Amt);
83 21
            $currency    = (string)$xmlEntry->Amt['Ccy'];
84 21
            $bookingDate = ((string) $xmlEntry->BookgDt->Dt) ?: (string) $xmlEntry->BookgDt->DtTm;
85 21
            $valueDate   = ((string) $xmlEntry->ValDt->Dt) ?: (string) $xmlEntry->ValDt->DtTm;
86 21
            $additionalInfo = ((string) $xmlEntry->AddtlNtryInf) ?: (string) $xmlEntry->AddtlNtryInf;
87
88 21
            if ((string) $xmlEntry->CdtDbtInd === 'DBIT') {
89 12
                $amount = $amount * -1;
90
            }
91
92 21
            $entry = new DTO\Entry(
93 21
                $record,
94 21
                $index,
95 21
                new Money($amount, new Currency($currency)),
96 21
                $this->dateDecoder->decode($bookingDate),
97 21
                $this->dateDecoder->decode($valueDate),
98 21
                $additionalInfo
99
            );
100
101 21
            if (isset($xmlEntry->RvslInd) && (string) $xmlEntry->RvslInd === 'true') {
102 1
                $entry->setReversalIndicator(true);
103
            }
104
105 21
            if (isset($xmlEntry->NtryRef) && (string) $xmlEntry->NtryRef) {
106 1
                $entry->setReference((string) $xmlEntry->NtryRef);
107
            }
108
109 21
            if (isset($xmlEntry->AcctSvcrRef) && (string) $xmlEntry->AcctSvcrRef) {
110 18
                $entry->setAccountServicerReference((string) $xmlEntry->AcctSvcrRef);
111
            }
112
113 21
            if (isset($xmlEntry->NtryDtls->Btch->PmtInfId) && (string) $xmlEntry->NtryDtls->Btch->PmtInfId) {
114 11
                $entry->setBatchPaymentId((string) $xmlEntry->NtryDtls->Btch->PmtInfId);
115
            }
116
117 21
            if (isset($xmlEntry->NtryDtls->TxDtls->Refs->PmtInfId) && (string) $xmlEntry->NtryDtls->TxDtls->Refs->PmtInfId) {
118 3
                $entry->setBatchPaymentId((string) $xmlEntry->NtryDtls->TxDtls->Refs->PmtInfId);
119
            }
120
121 21
            if (isset($xmlEntry->BkTxCd)) {
122 20
                $bankTransactionCode = new DTO\BankTransactionCode();
123
124 20
                if (isset($xmlEntry->BkTxCd->Prtry)) {
125 19
                    $proprietaryBankTransactionCode = new DTO\ProprietaryBankTransactionCode(
126 19
                        (string)$xmlEntry->BkTxCd->Prtry->Cd,
127 19
                        (string)$xmlEntry->BkTxCd->Prtry->Issr
128
                    );
129
130 19
                    $bankTransactionCode->setProprietary($proprietaryBankTransactionCode);
131
                }
132
133 20
                if (isset($xmlEntry->BkTxCd->Domn)) {
134 17
                    $domainBankTransactionCode = new DTO\DomainBankTransactionCode(
135 17
                        (string)$xmlEntry->BkTxCd->Domn->Cd
136
                    );
137
138 17
                    if (isset($xmlEntry->BkTxCd->Domn->Fmly)) {
139 17
                        $domainFamilyBankTransactionCode = new DTO\DomainFamilyBankTransactionCode(
140 17
                            (string)$xmlEntry->BkTxCd->Domn->Fmly->Cd,
141 17
                            (string)$xmlEntry->BkTxCd->Domn->Fmly->SubFmlyCd
142
                        );
143
144 17
                        $domainBankTransactionCode->setFamily($domainFamilyBankTransactionCode);
145
                    }
146
147 17
                    $bankTransactionCode->setDomain($domainBankTransactionCode);
148
                }
149
150 20
                $entry->setBankTransactionCode($bankTransactionCode);
151
            }
152
153 21
            if (isset($xmlEntry->Chrgs)) {
154
                $charges = new DTO\Charges();
155
156
                if (isset($xmlEntry->Chrgs->TtlChrgsAndTaxAmt) && (string) $xmlEntry->Chrgs->TtlChrgsAndTaxAmt) {
157
                    $amount      = StringToUnits::convert((string) $xmlEntry->Chrgs->TtlChrgsAndTaxAmt);
158
                    $currency    = (string)$xmlEntry->Chrgs->TtlChrgsAndTaxAmt['Ccy'];
159
160
                    $charges->setTotalChargesAndTaxAmount(new Money($amount, new Currency($currency)));
161
                }
162
163
                $chargesRecords = $xmlEntry->Chrgs->Rcrd;
164
                if ($chargesRecords) {
165
                    foreach ($chargesRecords as $chargesRecord) {
166
                        $chargesDetail = new DTO\ChargesRecord();
167
168
                        if (isset($chargesRecord->Amt) && (string) $chargesRecord->Amt) {
169
                            $amount      = StringToUnits::convert((string) $chargesRecord->Amt);
170
                            $currency    = (string)$chargesRecord->Amt['Ccy'];
171
172
                            if ((string) $chargesRecord->CdtDbtInd === 'DBIT') {
173
                                $amount = $amount * -1;
174
                            }
175
176
                            $chargesDetail->setAmount(new Money($amount, new Currency($currency)));
177
                        }
178
                        if (isset($chargesRecord->CdtDbtInd) && (string) $chargesRecord->CdtDbtInd === 'true') {
179
                            $chargesDetail->setChargesIncluded­Indicator(true);
180
                        }
181
                        if (isset($chargesRecord->Tp->Prtry->Id) && (string) $chargesRecord->Tp->Prtry->Id) {
182
                            $chargesDetail->setIdentification((string) $chargesRecord->Tp->Prtry->Id);
183
                        }
184
                        $charges->addRecord($chargesDetail);
185
                    }
186
                }
187
                $entry->setCharges($charges);
188
            }
189
190 21
            $this->entryDecoder->addTransactionDetails($entry, $xmlEntry);
191
192 21
            $record->addEntry($entry);
193 21
            $index++;
194
        }
195 22
    }
196
}
197