Completed
Push — master ( 4d3436...c8d1ff )
by Hanish
11s queued 10s
created

Record   A

Complexity

Total Complexity 42

Size/Duplication

Total Lines 193
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 14

Test Coverage

Coverage 79%

Importance

Changes 0
Metric Value
wmc 42
lcom 1
cbo 14
dl 0
loc 193
ccs 79
cts 100
cp 0.79
rs 9.0399
c 0
b 0
f 0

3 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 5 1
B addBalances() 0 35 7
F addEntries() 0 126 34

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