Conditions | 7 |
Paths | 12 |
Total Lines | 94 |
Lines | 0 |
Ratio | 0 % |
Changes | 0 |
Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.
For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.
Commonly applied refactorings include:
If many parameters/temporary variables are present:
1 | <?php |
||
44 | public function buildRequest($order) |
||
45 | { |
||
46 | $shippingAddress = null; |
||
|
|||
47 | if ($order->ShippingAddress()->exists()) { |
||
48 | $shippingAddress = $order->ShippingAddress(); |
||
49 | } else { |
||
50 | $shippingAddress = $order->BillingAddress(); |
||
51 | } |
||
52 | |||
53 | |||
54 | $mf = $this->getConnection(); |
||
55 | |||
56 | $request = $mf->withAccount( |
||
57 | [ |
||
58 | 'user_id' => $order->MemberID, |
||
59 | 'username_md5' => md5($order->Member()->Email), |
||
60 | ] |
||
61 | )->withEvent( |
||
62 | [ |
||
63 | 'transaction_id' => (string)$order->ID, |
||
64 | 'time' => date("c", strtotime($order->Created)), //see: https://stackoverflow.com/questions/22296712/convert-datetime-to-rfc-3339 should we use Created or LastEdited? |
||
65 | 'type' => 'purchase', |
||
66 | ] |
||
67 | )->withEmail( |
||
68 | [ |
||
69 | 'address' => $order->Member()->Email, |
||
70 | 'domain' => substr(strrchr($order->Member()->Email, "@"), 1), |
||
71 | ] |
||
72 | )->withBilling( |
||
73 | [ |
||
74 | 'first_name' => $order->BillingAddress()->FirstName, |
||
75 | 'last_name' => $order->BillingAddress()->Surname, |
||
76 | 'address' => $order->BillingAddress()->Address, |
||
77 | 'address_2' => $order->BillingAddress()->Address2, |
||
78 | 'city' => $order->BillingAddress()->City, |
||
79 | //'region' => '', // see: https://en.wikipedia.org/wiki/ISO_3166-2 |
||
80 | 'country' => $order->BillingAddress()->Country, |
||
81 | 'postal' => $order->BillingAddress()->PostalCode, |
||
82 | 'phone_number' => $order->BillingAddress()->Phone |
||
83 | ] |
||
84 | )->withShipping( |
||
85 | [ |
||
86 | 'first_name' => $shippingAddress->FirstName, |
||
87 | 'last_name' => $shippingAddress->Surname, |
||
88 | 'address' => $shippingAddress->Address, |
||
89 | 'address_2' => $shippingAddress->Address2, |
||
90 | 'city' => $shippingAddress->City, |
||
91 | //'region' => '', // see: https://en.wikipedia.org/wiki/ISO_3166-2 |
||
92 | 'country' => $shippingAddress->Country, |
||
93 | 'postal' => $shippingAddress->PostalCode, |
||
94 | 'phone_number' => $shippingAddress->Phone |
||
95 | ] |
||
96 | )->withOrder( |
||
97 | [ |
||
98 | 'amount' => $order->getTotal(), |
||
99 | 'currency' => $order->getTotalAsMoney()->currency, |
||
100 | //'discount_code' => '', //do we want to use this? |
||
101 | 'is_gift' => false, |
||
102 | 'has_gift_message' => false, |
||
103 | 'referrer_uri' => Director::absoluteURL('/'), |
||
104 | ] |
||
105 | ); |
||
106 | |||
107 | |||
108 | $deviceDetails = OrderStatusLog_DeviceDetails::get()->filter(['OrderID' => $order->ID])->first(); |
||
109 | if ($deviceDetails && $deviceDetails->exists()) { |
||
110 | $request = $request->withDevice( |
||
111 | [ |
||
112 | 'ip_address' => $deviceDetails->IPAddress, |
||
113 | 'user_agent' => $deviceDetails->UserAgent, |
||
114 | 'accept_language' => $deviceDetails->AcceptLanguage, |
||
115 | 'session_id' => $deviceDetails->SessionID, |
||
116 | ] |
||
117 | ); |
||
118 | } |
||
119 | |||
120 | |||
121 | foreach ($order->Items() as $orderItem) { |
||
122 | $itemID = $orderItem->BuyableID; |
||
123 | $product = DataObject::get_by_id($orderItem->BuyableClassName, $orderItem->BuyableID); |
||
124 | if ($product && $product->exists()) { |
||
125 | $itemID = $product->InternalItemID; |
||
126 | } |
||
127 | $request = $request->withShoppingCartItem( |
||
128 | [ |
||
129 | 'item_id' => (string)$itemID, |
||
130 | 'quantity' => (int)$orderItem->Quantity, |
||
131 | 'price' => $orderItem->CalculatedTotal, |
||
132 | ] |
||
133 | ); |
||
134 | } |
||
135 | |||
136 | return $request; |
||
137 | } |
||
138 | |||
186 |
This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.
Both the
$myVar
assignment in line 1 and the$higher
assignment in line 2 are dead. The first because$myVar
is never used and the second because$higher
is always overwritten for every possible time line.