This code can woocommerce order change status. But as you can see, this code is hard to read. How can I make it cleaner?
<?php
$tarih = explode('T', $order->get_date_completed());
$now = date("Y-m-d");
$origin = date_create($tarih[0]);
$target = date_create($now);
$interval = date_diff($origin, $target);
$iade_hakki=$interval->d;
$aide_yazi='';
?>
<section class="woocommerce-columns woocommerce-columns--2 woocommerce-columns--addresses col2-set addresses">
<div class="woocommerce-column woocommerce-column--1 woocommerce-column--billing-address col-1">
<?php
global $wp;
$current_url = home_url(add_query_arg(array(), $wp->request));
if ($order->get_status()=="completed") {
if ($iade_hakki>=14){ ?>
<?php } else{?>
<button id="button-iade" data-id="<?=$order_id?>" style="border-radius: 4px;border:1px solid #92cecc; background-color: white;color:#92cecc" data-redirect="<?=$current_url?>" class="woocommerce-Button button woocommerce-column__title" >
Siparişi İade Et </button>
<?php }
}
elseif ($order->get_status()=='shipped'){ ?>
<button id="button-iade" style="border-radius: 4px;border:1px solid #92cecc; background-color: white;color:#92cecc" data-id="<?=$order_id?>" data-redirect="<?=$current_url?>" class="woocommerce-Button button woocommerce-column__title" >
Siparişi İade Et </button>
<?php }
else if ($order->get_status() == 'processing' || $order->get_status() == 'on-hold') {
global $wp;
$current_url = home_url(add_query_arg(array(), $wp->request));
?>
<button id="button-iptal" style="border-radius: 4px;border:1px solid #92cecc; background-color: white;color:#92cecc" data-id="<?=$order_id?>" data-redirect="<?=$current_url?>" class="woocommerce-Button button woocommerce-column__title" >
Siparişi İptal Et </button>
<?php } else{
echo '';
}
?>
2 Answers 2
To extract the substring before
T
, use the most direct non-regex tool:$dateCompleted = strstr($order->get_date_completed(), 'T', true);
Your code unnecessarily creates an array of which you only need the first element. Furthermore,
explode()
is not given a limit parameter, so it needlessly reads the whole string looking for multipleT
s. So long as the incoming date expression always has the same format, usestrstr()
.->d
is not a good idea. This does not give you the total days in the diff -- it gives you the days AFTER all of the larger date units have extracted their values. https://3v4l.org/BsSgK Instead, use->days
.
Beyond those pieces of advice, I agree with all of @Anonymous's advice and recommend that you study the PSR-12 coding standards which will help you to make your code easier to read and maintain.
This is a partial review only, I may edit this post later as more details become available.
I am not familiar with woocommerce at all. You may have design constraints but I would strongly recommend to use a templating engine like twig - the idea is to separate code (logic) from pages (layout).
The code for a number of functions is not present in your post eg: home_url, add_query_arg, get_status. Thus we cannot appreciate the full scope of your code.
I don't know what iade_hakki means in this context, the pitfall of not using English for naming objects (and comments) is that the code is hard to comprehend for someone who is not a native speaker of your language. This may be a hindrance to adoption by third parties if you decide to distribute or open-source your code at some point. And obviously it makes maintenance harder if someone else "inherits" your code in the future.
And speaking of comments there are none. Yet they would be welcome to clarify the purpose and mode of operation of this script. Quality code should speak for itself but should still be commented. A few lines could suffice.
I would get rid of the inline CSS too, for example:
<button id="button-iade" style="border-radius: 4px;border:1px solid #92cecc; background-color: white;color:#92cecc" data-id="<?=$order_id?>" data-redirect="<?=$current_url?>" class="woocommerce-Button button woocommerce-column__title" >
Instead I would either add some more classes or possibly override the main style sheet if there is one. If you want to change your layout later, you'll have to edit a potentially large number of files, which is a cumbersome process. If everything is centralized in a style sheet, it becomes simple.
Note: when I design websites I like to provide at least one alternative style sheet, for example a high-contrast style sheet suitable for visually impaired people. To be able to easily switch from one style sheet to another it would be better not to have inline CSS inside your files.
There is a lack of consistency in the naming of CSS classes eg:
class="woocommerce-Button button woocommerce-column__title"
Consider the following:
- mixed case
- mix of hyphens and underscore
- double underscore: why ?
Another example:
<div class="woocommerce-column woocommerce-column--1 woocommerce-column--billing-address col-1">
Double hyphens. Why ?
There is a high risk of typos/confusion, so prefer lowercase and single hyphens. If you prefer underscores, stick with them but stay consistent.
This should be reasonable:
class="button woocommerce-button woocommerce-column-title"
Discussion: Why are dashes preferred for CSS selectors / HTML attributes?
14 is a magic number and could be made a constant. But what does that number represent ? It deserves to be commented.
if ($iade_hakki>=14){ ?>
Indentation can be improved, so that control blocks (if, loops etc) are outlined better visually. Spacing too. This:
}
elseif ($order->get_status()=='shipped'){ ?>
could be written:
} elseif ($order->get_status() == 'shipped') { ?>
Instead of if/elseif I would use a switch/case block instead... So:
switch ($order->get_status()) {
case 'completed':
// do something
break;
case 'shipped':
// do something
break;
case 'processing':
case 'on-hold':
// do something
break;
}
Note that 'processing' and 'on-hold' a grouped together.
And in fact you have repetitive code:
<button id="button-iade" data-id="<?=$order_id?>" style="border-radius: 4px;border:1px solid #92cecc; background-color: white;color:#92cecc" data-redirect="<?=$current_url?>" class="woocommerce-Button button woocommerce-column__title" >
This is the same code, all three lines seem identical, only the id varies. So it makes sense to generate this line only once, at the end of your block ?