Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like Shoppingcart 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 Shoppingcart, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
29 | class Shoppingcart extends Base |
||
30 | { |
||
31 | /** |
||
32 | * @var \HaaseIT\Toolbox\Textcat |
||
33 | */ |
||
34 | private $textcats; |
||
35 | |||
36 | /** |
||
37 | * @var array |
||
38 | */ |
||
39 | private $imagestosend = []; |
||
40 | |||
41 | /** |
||
42 | * Shoppingcart constructor. |
||
43 | * @param \Zend\ServiceManager\ServiceManager $serviceManager |
||
44 | */ |
||
45 | public function __construct(\Zend\ServiceManager\ServiceManager $serviceManager) |
||
46 | { |
||
47 | parent::__construct($serviceManager); |
||
48 | $this->textcats = $this->serviceManager->get('textcats'); |
||
49 | } |
||
50 | |||
51 | /** |
||
52 | * |
||
53 | */ |
||
54 | public function preparePage() |
||
|
|||
55 | { |
||
56 | $this->P = new \HaaseIT\HCSF\CorePage($this->serviceManager); |
||
57 | $this->P->cb_pagetype = 'contentnosubnav'; |
||
58 | |||
59 | if (HelperConfig::$shop['show_pricesonlytologgedin'] && !CHelper::getUserData()) { |
||
60 | $this->P->oPayload->cl_html = $this->textcats->T('denied_notloggedin'); |
||
61 | } else { |
||
62 | $this->P->cb_customcontenttemplate = 'shop/shoppingcart'; |
||
63 | |||
64 | // Check if there is a message to display above the shoppingcart |
||
65 | $this->P->oPayload->cl_html = $this->getNotification(); |
||
66 | |||
67 | // Display the shoppingcart |
||
68 | if (isset($_SESSION['cart']) && count($_SESSION['cart']) >= 1) { |
||
69 | $aErr = []; |
||
70 | if (filter_input(INPUT_POST, 'doCheckout') === 'yes') { |
||
71 | $aErr = $this->validateCheckout($aErr); |
||
72 | if (count($aErr) === 0) { |
||
73 | \HaaseIT\HCSF\Helper::redirectToPage($this->doCheckout()); |
||
74 | } |
||
75 | } |
||
76 | |||
77 | $aShoppingcart = SHelper::buildShoppingCartTable($_SESSION['cart'], false, '', $aErr); |
||
78 | |||
79 | $this->P->cb_customdata = $aShoppingcart; |
||
80 | } else { |
||
81 | $this->P->oPayload->cl_html .= $this->textcats->T('shoppingcart_empty'); |
||
82 | } |
||
83 | } |
||
84 | } |
||
85 | |||
86 | /** |
||
87 | * @param array $aErr |
||
88 | * @return array |
||
89 | */ |
||
90 | private function validateCheckout($aErr = []) |
||
91 | { |
||
92 | $aErr = CHelper::validateCustomerForm(HelperConfig::$lang, $aErr, true); |
||
93 | View Code Duplication | if (!CHelper::getUserData() && filter_input(INPUT_POST, 'tos') !== 'y') { |
|
94 | $aErr['tos'] = true; |
||
95 | } |
||
96 | View Code Duplication | if (!CHelper::getUserData() && filter_input(INPUT_POST, 'cancellationdisclaimer') !== 'y') { |
|
97 | $aErr['cancellationdisclaimer'] = true; |
||
98 | } |
||
99 | $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod'); |
||
100 | if ( |
||
101 | $postpaymentmethod === null |
||
102 | || in_array($postpaymentmethod, HelperConfig::$shop['paymentmethods'], true) === false |
||
103 | ) { |
||
104 | $aErr['paymentmethod'] = true; |
||
105 | } |
||
106 | |||
107 | return $aErr; |
||
108 | } |
||
109 | |||
110 | /** |
||
111 | * @param $aV |
||
112 | * @return array |
||
113 | */ |
||
114 | private function getItemImage($aV) |
||
115 | { |
||
116 | // base64 encode img and prepare for db |
||
117 | // image/png image/jpeg image/gif |
||
118 | // data:{mimetype};base64,XXXX |
||
1 ignored issue
–
show
|
|||
119 | |||
120 | $aImagesToSend = []; |
||
121 | $base64Img = false; |
||
122 | $binImg = false; |
||
123 | |||
124 | if (HelperConfig::$shop['email_orderconfirmation_embed_itemimages_method'] === 'glide') { |
||
125 | $sPathToImage = '/'.HelperConfig::$core['directory_images'].'/'.HelperConfig::$shop['directory_images_items'].'/'; |
||
126 | $sImageroot = PATH_BASEDIR . HelperConfig::$core['directory_glide_master']; |
||
127 | |||
128 | if ( |
||
129 | is_file($sImageroot.substr($sPathToImage.$aV['img'], strlen(HelperConfig::$core['directory_images']) + 1)) |
||
130 | && $aImgInfo = getimagesize($sImageroot.substr($sPathToImage.$aV['img'], strlen(HelperConfig::$core['directory_images']) + 1)) |
||
131 | ) { |
||
132 | $glideserver = \League\Glide\ServerFactory::create([ |
||
133 | 'source' => $sImageroot, |
||
134 | 'cache' => PATH_GLIDECACHE, |
||
135 | 'max_image_size' => HelperConfig::$core['glide_max_imagesize'], |
||
136 | ]); |
||
137 | $glideserver->setBaseUrl('/' . HelperConfig::$core['directory_images'] . '/'); |
||
138 | $base64Img = $glideserver->getImageAsBase64($sPathToImage.$aV['img'], HelperConfig::$shop['email_orderconfirmation_embed_itemimages_glideparams']); |
||
139 | $TMP = explode(',', $base64Img); |
||
140 | $binImg = base64_decode($TMP[1]); |
||
141 | unset($TMP); |
||
142 | } |
||
143 | } else { |
||
144 | $sPathToImage = |
||
145 | PATH_DOCROOT.HelperConfig::$core['directory_images'].'/' |
||
146 | .HelperConfig::$shop['directory_images_items'].'/' |
||
147 | .HelperConfig::$shop['directory_images_items_email'].'/'; |
||
148 | if ($aImgInfo = getimagesize($sPathToImage.$aV['img'])) { |
||
149 | $binImg = file_get_contents($sPathToImage.$aV['img']); |
||
150 | $base64Img = 'data:' . $aImgInfo['mime'] . ';base64,'; |
||
151 | $base64Img .= base64_encode($binImg); |
||
152 | } |
||
153 | } |
||
154 | if (HelperConfig::$shop['email_orderconfirmation_embed_itemimages']) { |
||
155 | $aImagesToSend['binimg'] = $binImg; |
||
156 | } |
||
157 | if ($base64Img) { |
||
158 | $aImagesToSend['base64img'] = $base64Img; |
||
159 | } |
||
160 | return $aImagesToSend; |
||
161 | } |
||
162 | |||
163 | /** |
||
164 | * @return array |
||
165 | */ |
||
166 | private function prepareDataOrder() |
||
167 | { |
||
168 | $cartpricesums = $_SESSION['cartpricesums']; |
||
169 | return [ |
||
170 | 'o_custno' => filter_var(trim(Tools::getFormfield('custno')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
171 | 'o_email' => filter_var(trim(Tools::getFormfield('email')), FILTER_SANITIZE_EMAIL), |
||
172 | 'o_corpname' => filter_var(trim(Tools::getFormfield('corpname')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
173 | 'o_name' => filter_var(trim(Tools::getFormfield('name')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
174 | 'o_street' => filter_var(trim(Tools::getFormfield('street')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
175 | 'o_zip' => filter_var(trim(Tools::getFormfield('zip')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
176 | 'o_town' => filter_var(trim(Tools::getFormfield('town')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
177 | 'o_phone' => filter_var(trim(Tools::getFormfield('phone')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
178 | 'o_cellphone' => filter_var(trim(Tools::getFormfield('cellphone')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
179 | 'o_fax' => filter_var(trim(Tools::getFormfield('fax')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
180 | 'o_country' => filter_var(trim(Tools::getFormfield('country')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
181 | 'o_group' => trim(CHelper::getUserData('cust_group')), |
||
182 | 'o_remarks' => filter_var(trim(Tools::getFormfield('remarks')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
183 | 'o_tos' => (filter_input(INPUT_POST, 'tos') === 'y' || CHelper::getUserData()) ? 'y' : 'n', |
||
184 | 'o_cancellationdisclaimer' => (filter_input(INPUT_POST, 'cancellationdisclaimer') === 'y' || CHelper::getUserData()) ? 'y' : 'n', |
||
185 | 'o_paymentmethod' => filter_var(trim(Tools::getFormfield('paymentmethod')), FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
186 | 'o_sumvoll' => $cartpricesums['sumvoll'], |
||
187 | 'o_sumerm' => $cartpricesums['sumerm'], |
||
188 | 'o_sumnettoall' => $cartpricesums['sumnettoall'], |
||
189 | 'o_taxvoll' => $cartpricesums['taxvoll'], |
||
190 | 'o_taxerm' => $cartpricesums['taxerm'], |
||
191 | 'o_sumbruttoall' => $cartpricesums['sumbruttoall'], |
||
192 | 'o_mindermenge' => isset($cartpricesums['mindergebuehr']) ? $cartpricesums['mindergebuehr'] : '', |
||
193 | 'o_shippingcost' => SHelper::getShippingcost(), |
||
194 | 'o_orderdate' => date('Y-m-d'), |
||
195 | 'o_ordertimestamp' => time(), |
||
196 | 'o_authed' => CHelper::getUserData() ? 'y' : 'n', |
||
197 | 'o_sessiondata' => serialize($_SESSION), |
||
198 | 'o_postdata' => serialize($_POST), |
||
199 | 'o_remote_address' => filter_input(INPUT_SERVER, 'REMOTE_ADDR', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
200 | 'o_ordercompleted' => 'n', |
||
201 | 'o_paymentcompleted' => 'n', |
||
202 | 'o_srv_hostname' => filter_input(INPUT_SERVER, 'SERVER_NAME', FILTER_SANITIZE_STRING, FILTER_FLAG_STRIP_LOW), |
||
203 | 'o_vatfull' => HelperConfig::$shop['vat']['full'], |
||
204 | 'o_vatreduced' => HelperConfig::$shop['vat']['reduced'], |
||
205 | ]; |
||
206 | } |
||
207 | |||
208 | /** |
||
209 | * @param int $orderid |
||
210 | * @param string $cartkey |
||
211 | * @param array $values |
||
212 | * @return array |
||
213 | */ |
||
214 | private function buildOrderItemRow($orderid, $cartkey, array $values) |
||
215 | { |
||
216 | return [ |
||
217 | 'oi_o_id' => $orderid, |
||
218 | 'oi_cartkey' => $cartkey, |
||
219 | 'oi_amount' => $values['amount'], |
||
220 | 'oi_price_netto_list' => $values['price']['netto_list'], |
||
221 | 'oi_price_netto_use' => $values['price']['netto_use'], |
||
222 | 'oi_price_brutto_use' => $values['price']['brutto_use'], |
||
223 | 'oi_price_netto_sale' => isset($values['price']['netto_sale']) ? $values['price']['netto_sale'] : '', |
||
224 | 'oi_price_netto_rebated' => isset($values['price']['netto_rebated']) ? $values['price']['netto_rebated'] : '', |
||
225 | 'oi_vat' => HelperConfig::$shop['vat'][$values['vat']], |
||
226 | 'oi_rg' => $values['rg'], |
||
227 | 'oi_rg_rebate' => isset( |
||
228 | HelperConfig::$shop['rebate_groups'][$values['rg']][trim(CHelper::getUserData('cust_group'))] |
||
229 | ) |
||
230 | ? HelperConfig::$shop['rebate_groups'][$values['rg']][trim(CHelper::getUserData('cust_group'))] |
||
231 | : '', |
||
232 | 'oi_itemname' => $values['name'], |
||
233 | 'oi_img' => $this->imagestosend[$values['img']]['base64img'], |
||
234 | ]; |
||
235 | } |
||
236 | |||
237 | private function writeCheckoutToDB() |
||
238 | { |
||
239 | /** @var \Doctrine\DBAL\Connection $dbal */ |
||
240 | $dbal = $this->serviceManager->get('dbal'); |
||
241 | |||
242 | try { |
||
243 | $dbal->beginTransaction(); |
||
244 | |||
245 | $aDataOrder = $this->prepareDataOrder(); |
||
246 | |||
247 | $iInsertID = \HaaseIT\HCSF\Helper::autoInsert($dbal, 'orders', $aDataOrder); |
||
248 | |||
249 | foreach ($_SESSION['cart'] as $sK => $aV) { |
||
250 | $this->imagestosend[$aV['img']] = $this->getItemImage($aV); |
||
251 | |||
252 | \HaaseIT\HCSF\Helper::autoInsert( |
||
253 | $dbal, |
||
254 | 'orders_items', |
||
255 | $this->buildOrderItemRow($iInsertID, $sK, $aV) |
||
256 | ); |
||
257 | } |
||
258 | $dbal->commit(); |
||
259 | |||
260 | return $iInsertID; |
||
261 | } catch (\Exception $e) { |
||
262 | // If something raised an exception in our transaction block of statements, |
||
263 | // roll back any work performed in the transaction |
||
264 | print '<p>Unable to complete transaction!</p>'; |
||
265 | error_log($e); |
||
266 | $dbal->rollBack(); |
||
267 | } |
||
268 | } |
||
269 | |||
270 | private function doCheckout() |
||
271 | { |
||
272 | $iInsertID = $this->writeCheckoutToDB(); |
||
273 | |||
274 | $sMailbody_us = $this->buildOrderMailBody(false, $iInsertID); |
||
275 | $sMailbody_they = $this->buildOrderMailBody(true, $iInsertID); |
||
276 | |||
277 | // write to file |
||
278 | $this->writeCheckoutToFile($sMailbody_us); |
||
279 | |||
280 | // Send Mails |
||
281 | $this->sendCheckoutMails($iInsertID, $sMailbody_us, $sMailbody_they); |
||
282 | |||
283 | unset($_SESSION['cart'], $_SESSION['cartpricesums'], $_SESSION['sondercart']); |
||
284 | |||
285 | $postpaymentmethod = filter_input(INPUT_POST, 'paymentmethod'); |
||
286 | if ($postpaymentmethod !== null) { |
||
287 | if ( |
||
288 | $postpaymentmethod === 'paypal' |
||
289 | && isset(HelperConfig::$shop['paypal_interactive']) |
||
290 | && HelperConfig::$shop['paypal_interactive'] |
||
291 | ) { |
||
292 | return '/_misc/paypal.html?id=' . $iInsertID; |
||
293 | } elseif ($postpaymentmethod === 'sofortueberweisung') { |
||
294 | return '/_misc/sofortueberweisung.html?id=' . $iInsertID; |
||
295 | } |
||
296 | } |
||
297 | |||
298 | return '/_misc/checkedout.html?id=' . $iInsertID; |
||
299 | } |
||
300 | |||
301 | /** |
||
302 | * @param int $iInsertID |
||
303 | * @param string $sMailbody_us |
||
304 | * @param string $sMailbody_they |
||
305 | */ |
||
306 | private function sendCheckoutMails($iInsertID, $sMailbody_us, $sMailbody_they) |
||
307 | { |
||
308 | if ( |
||
309 | isset(HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' .HelperConfig::$lang]) |
||
310 | && file_exists( |
||
311 | PATH_DOCROOT.HelperConfig::$core['directory_emailattachments'] |
||
312 | .'/'.HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' |
||
313 | .HelperConfig::$lang] |
||
314 | ) |
||
315 | ) { |
||
316 | $aFilesToSend[] = |
||
317 | PATH_DOCROOT.HelperConfig::$core['directory_emailattachments'].'/' |
||
318 | .HelperConfig::$shop['email_orderconfirmation_attachment_cancellationform_' .HelperConfig::$lang]; |
||
319 | } else { |
||
320 | $aFilesToSend = []; |
||
321 | } |
||
322 | |||
323 | Helper::mailWrapper( |
||
324 | filter_input(INPUT_POST, 'email', FILTER_SANITIZE_EMAIL), |
||
325 | $this->textcats->T('shoppingcart_mail_subject') . ' ' . $iInsertID, |
||
326 | $sMailbody_they, |
||
327 | $this->imagestosend, |
||
328 | $aFilesToSend |
||
329 | ); |
||
330 | Helper::mailWrapper( |
||
331 | HelperConfig::$core['email_sender'], |
||
332 | 'Bestellung im Webshop Nr: ' . $iInsertID, |
||
333 | $sMailbody_us, |
||
334 | $this->imagestosend |
||
335 | ); |
||
336 | } |
||
337 | |||
338 | /** |
||
339 | * @param string $sMailbody_us |
||
340 | */ |
||
341 | private function writeCheckoutToFile($sMailbody_us) |
||
348 | |||
349 | /** |
||
350 | * @param string $field |
||
351 | * @return string |
||
352 | */ |
||
353 | private function getPostValue($field) |
||
358 | |||
359 | /** |
||
360 | * @param bool $bCust |
||
361 | * @param int $iId |
||
362 | * @return mixed |
||
363 | */ |
||
364 | private function buildOrderMailBody($bCust = true, $iId) |
||
416 | |||
417 | /** |
||
418 | * @return string |
||
419 | */ |
||
420 | private function getNotification() |
||
457 | } |
||
458 |
Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: