2
\$\begingroup\$

I use the following code: fetchAll and for($mich=0;$mich<10;$mich++).

Is this the best way?

Is there a better way, either to optimize or replace this code?

<?php
include("cache/phpfastcache.php");
phpFastCache::setup("storage","auto");
$cache = phpFastCache();
$products = $cache->get("product_page");
if($products == null) {
    $akhbarkotah1 = $conn->prepare("SELECT text,time FROM small WHERE active='0' ORDER BY time DESC LIMIT 10");
    $akhbarkotah1->execute();
    $products = $akhbarkotah1->fetchAll();
    $cache->set("product_page", $products,30);
}
for($mich=0;$mich<10;$mich++){
    $text1 =$products[$mich][0];
    $time1 =$products[$mich][1];
    ?>
    <a class="lastnews">
        <div id="lastnews_title" class="YekanBlack10" style="width:585px;">
            <div><?php echo $text1; ?></div>
            <div class="YekanRed10" style="text-align:left">
                <?php echo timeTonow($time1); ?>
            </div>
            <div style="border-bottom:2px solid #7c0000;height:5px;"></div>
        </div>
    </a>
    <?php
}
?>
\$\endgroup\$
3
  • \$\begingroup\$ Does your actual code have this [non-]indentation? \$\endgroup\$ Mar 3, 2014 at 21:34
  • \$\begingroup\$ i indent code , its not important \$\endgroup\$ Mar 3, 2014 at 21:48
  • 7
    \$\begingroup\$ it actually is important, if you want readable/maintainable code. Indentation plays a very important part in making code structure obvious. \$\endgroup\$ Mar 3, 2014 at 21:55

1 Answer 1

3
\$\begingroup\$

Using fetchAll usually is not a problem. There are some things to optimize here in my opinion:

  • Use a foreach loop instead of for loop. Your result is already limited. No need to duplicate this here. This removes the iterator counter and assignment:

    foreach ($products as $product) {
        $text1 = $product[0];
        $time1 = $product[1];
    ?>
    
  • Using the Object fetch mode (returns objects instead of indexed arrays) makes this even more readable and saves you the name assignments:

    foreach ($products as $product) {
         // $product->text1; $product->time1;
    ?>
    
  • Though I'd recommend to remove these. Introducing numbers in variable names usually suggests there is a second one. E.g. time2.

  • As @mat's mug pointed out, indentation is very important. Same goes for empty lines. There are great to structure your code in logical blocks. For this example here, I'd have an empty line after setting up the cache and one after loading the products.
\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much , can you help me for change to object fetch? i change $akhbarkotah1->fetchAll(); to $akhbarkotah1->fetch(PDO::FETCH_OBJ); and $text1 = $product[0]; to $text =$product->text; is this true? \$\endgroup\$ Mar 4, 2014 at 14:46
  • \$\begingroup\$ fetch returns a single entry only. You want all entries of the result-set: fetchAll should be used here (same argument though). This will return you an array of objects, one object for every row of the result-set. For detailed questions and howtos StackOverflow is the right place to ask :) \$\endgroup\$
    – Fge
    Mar 4, 2014 at 14:55
  • \$\begingroup\$ i ask this problem in : stackoverflow.com/questions/22176016/… \$\endgroup\$ Mar 4, 2014 at 15:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.