Fetch and display products from a SOAP call

Question 1

I’m relatively new to php and any advice is appreciated.

I retrieve some data from an external source and process them in a foreach loop to display the items. Now I’m not sure if the way I choose is the right way or if there is a better way for a situation like this.

And here the code I use:

define ("WSDL_SERVER",
"XXX");
$oSmartFeed= new SoapClient(WSDL_SERVER);
$oSessionHash= $oSmartFeed->login('XXX', 'XXX');
if(!$oSessionHash->HasError){
$sSessionHash= $oSessionHash->Records['sessionHash'];
$aConfig= array('limit'=>5, 'offset'=>2, 'feeds'=>[XXX]);
$aResult= $oSmartFeed-> getProductData ($sSessionHash, $aConfig);

echo '<ul>';

foreach ($aResult->Records AS $aRecord) {

    $produktName = $aRecord['productname'];
    $imageUrl = $aRecord['imagebigurl'];
    $price = $aRecord['currentprice'] . $aRecord['currency'];
    $affLink = $aRecord['deeplinkurl'];
    $productDescription = $aRecord['productdescriptionlong'];
    $versandKosten = $aRecord['shipping'] . ' €';

    ?>
       <li>
           <a href="<?php echo $affLink;?>">
               <img src="<?php echo $imageUrl; ?>" alt="<?php echo $produktName; ?>">
               <h3>
                   <?php echo $produktName; ?>
               </h3>
               <p>
                   <?php echo $productDescription;?>
               </p>
               <aside>
                   <span>
                       Price: <?php echo $price; ?>
                   </span>
                   <br>
                   <span>
                       Shipping: <?php echo $versandKosten; ?>
                   </span>
               </aside>
           </a>
       </li>
    <?php
}
echo '</ul>';
$oSmartFeed->logout($sSessionHash);

Question 2

Have you considered using a template engine vs. writing old-style PHP spaghetti code?

if(!$oSessionHash->HasError){

Reverse this conditional to get rid of the huge section of code within this conditional (whihc is actually not even closed with the code shown. So something like:

if($oSessionHash->HasError) {
    // die, exit, redirect or similar
}
// rest of code, now no longer in a conditional.

This strategy of existing early from scripts/function/methods will help you write much less fragile code.

A number of stylistic concerns:

Drop the whole $o*, $a*, $s* variable naming (which looks like you are trying to infer variable type in name). Your variables should have meaningful and specific names which convey their meaning. Also, if you use a good IDE, you won’t need this.
Your spacing around assgniment operators, flow control structures, etc. is wildly inconsisten, making your code hard to read. If you are strating in PHP, I would recommend getting to know the PSR-1 and PSR-2 coding standards as well as other PSR recommendations which are most widely accetped ways of writing code in the profession PHP development community. A good IDE will be able to help you enforce these styles.
Don’t split your define across two lines unnecessarily like you are.

Why not perform logout immediately after your API call response is returned, rather than wait until after displaying results?

This code is very happy path oriented. What happens if API doesn’t respond? What if it responds with an error code? What if it returns empty result set?

Leave a Reply Cancel reply