Displaying an order status form

Question

Below is code i used to call Api and display the "Status" results in web page.

my senior told me that code is completely mess and need to improve so that it should be easy to understand for others. but i am not getting how i can refactor below code ? is mixing php and html code is bad ? i really need to mix those html and php to get the results that i needed .please help me for this....

<p><button type= "button" class="call">Show Status</button></p>

    <table class="tbl-qa" border="1">
        <thead>
            <tr>    
               <th class="table-header"></th>     
               <th class="table-header">ORDERID</th>                                                      
               <th class="table-header">Status</th>                    
            </tr>
        </thead>

       <tbody id="table-body">
<?php
$tabindex = 1;

if (!empty($orderrecords))
    {
    foreach($orderrecords as $k => $v)
        { ?>

        <?php
        $hide = '';
        $data['username'] = 'admin';
        $data['password'] = 'admin123';     

        $url = 'https://plapi.ecomexpress.in/track_me/api/mawbd/?awb=awbnumber&order=' . $orderrecords[$k]["order_id"] . '&username=admin&password=admin123';
        $ch = curl_init();
        // some curl code
        $res = explode("\n", $output);
        if (!isset($res[13]))
            {
            $res[13] = null;
            }

        $status = $res[13];
?>

<tr class="table-row" id="table-row-<?php echo $orderrecords[$k]["id"]; ?>" tabindex="<?php echo $tabindex; ?>">

<td><input onclick="assignorderids('<?php echo $orderrecords[$k]["order_id"]; ?>')" type="checkbox" name="assigneeid" id="assigneeid-<?php echo $orderrecords[$k]["order_id"]; ?>" value="<?php echo $orderrecords[$k]["order_id"]; ?>"></td>

<td><?php echo $orderrecords[$k]["order_id"]; ?></td>             
<td><?php echo $status; ?></td>

</tr>


 <?php
 $tabindex++;
 }
    } ?>

    </tbody>
    </table>

@Toto he is not permanent employee working in our company , he usually visits some time in a week and check code of all beginner developers and tell us to improve, can you please help me for this..... — vickey colors
– vickey colors, Commented Feb 22, 2018 at 15:04

thiagoalessio · Accepted Answer · 2018-02-23 20:53:17Z

I can totally understand the frustration of your colleague that will need to maintain this code.

But would have been more constructive to tell that this code is considered a "mess" mainly because:

Logic of fetching order status is mixed in the middle of table rendering;
"Dirty" formatting: inconsistent indentation and trailing spaces;

The exact same thing could be better expressed like that:

<?php
function getOrderTrackingStatus($orderId) {
    $url = 'https://plapi.ecomexpress.in/track_me/api/mawbd/';
    $url.= "?awb=awbnumber&order=$orderId&username=admin&password=admin123";

    $ch = curl_init();
    // some curl code
    $res = explode("\n", $output);

    $status = isset($res[13]) ? $res[13] : null;
    return $status;
}
?>

<p>
    <button type="button" class="call">Show Status</button>
</p>

<table class="tbl-qa" border="1">
    <thead>
        <tr>
            <th class="table-header"></th>
            <th class="table-header">ORDERID</th>
            <th class="table-header">Status</th>
        </tr>
    </thead>

    <tbody id="table-body">
    <?php $tabindex = 1; ?>
    <?php foreach($orderrecords as $record): ?>

        <tr class="table-row"
            id="table-row-<?php echo $record['id']; ?>"
            tabindex="<?php echo $tabindex++; ?>">
            <td>
                <input type="checkbox"
                       name="assigneeid"
                       id="assigneeid-<?php echo $record['order_id']; ?>"
                       value="<?php echo $record['order_id']; ?>"
                       onclick="assignorderids('<?php echo $record['order_id']; ?>')">
            </td>
            <td><?php echo $orderId; ?></td>
            <td><?php echo getOrderTrackingStatus($orderId); ?></td>
        </tr>

    <?php endforeach; ?>
    </tbody>
</table>

Ideally this getOrderTrackingStatus function would be moved somewhere else, instead of living inside the same file that handles data visualisation.

Hope it helps you, and depending on your project you should consider using a templating engine as a way to force yourself to move logic code away from the views.

Stack Exchange Network

Displaying an order status form

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Displaying an order status form

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions