0

I have a custom code in sidebar.php that shows all my categories and number of posts+custom posts. The code is working fine but, since I copied/pasted and modified various other codes from stackexchange forum, I am wondering how efficient the code is.

I don't have any background on php, but at least I can tell the below code is little bit messy. Especially, the code repeats almost same lines three times to show all child categories properly and count normal posts and custom posts separately. The codes shows something like

Grandparent (100+20)

  • Parent1 (20+5)
    • Child 1.1 (10+0)
    • Child 1.2 (10+5)
  • Parent 2 (80+15)
    • Child 2.1 (40+5)
    • Child 2.2 (40+5)
    • Child 2.3 (0+5)

Is there a way to simplify the code below? and how good/bad is the code in term of speed+efficiency?

<aside class="widget widget_categories">
<h3 class="widget-title"><span class="widget-title-tab">Categories</span></h3>  
    <?php $categories = get_categories( array('parent' => 0, 'hide_empty' => 0, 'orderby' => 'term_order' ) ); ?>
    <?php if($categories): ?>
        <ul>
            <?php $cat_posts = get_posts('post_type=post&category=' . $cat->term_id . '&numberposts=-1'); ?>
            <?php $cat_probs = get_posts('post_type=probsoln&category=' . $cat->term_id . '&numberposts=-1'); ?>
            <?php $count = count($cat_posts); ?>
            <?php $count_prob = count($cat_probs); ?>
            <li class="cat-item" style=""><a href="<?php echo get_category_link($cat->term_id); ?>"><?php echo $cat->name; ?></a><span class="post_count"><?php echo " ($count+$count_prob)"; ?></span>
            <?php $sub_categories = get_categories( array('parent' => $cat->term_id, 'hide_empty' => 0, 'orderby' => 'term_order' ) ); ?>
            <?php if($sub_categories): ?>
                <ul class="children">
                <?php foreach ($sub_categories as $sub_cat): ?>
                    <?php $sub_cat_posts = get_posts('post_type=post&category=' . $sub_cat->term_id . '&numberposts=-1'); ?>
                    <?php $sub_cat_probs = get_posts('post_type=probsoln&category=' . $sub_cat->term_id . '&numberposts=-1'); ?>
                    <?php $sub_count = count($sub_cat_posts); ?>
                    <?php $sub_count_prob = count($sub_cat_probs); ?>
                        <li class="cat-item"><a href="<?php echo get_category_link($sub_cat->term_id); ?>"><?php echo $sub_cat->name; ?></a><span class="post_count"><?php echo " ($sub_count+$sub_count_prob)"; ?></span>
                        <?php $sub_sub_categories = get_categories( array('parent' => $sub_cat->term_id, 'hide_empty' => 0, 'orderby' => 'term_order' ) ); ?>
                        <?php if($sub_sub_categories): ?>
                            <ul class="children">
                            <?php foreach ($sub_sub_categories as $sub_sub_cat): ?>
                            <?php $sub_sub_cat_posts = get_posts('post_type=post&category=' . $sub_sub_cat->term_id . '&numberposts=-1'); ?>
                            <?php $sub_sub_cat_probs = get_posts('post_type=probsoln&category=' . $sub_sub_cat->term_id . '&numberposts=-1'); ?>
                            <?php $sub_sub_count = count($sub_sub_cat_posts); ?>
                            <?php $sub_sub_count_prob = count($sub_sub_cat_probs); ?>   
                                <li class="cat-item"><a href="<?php echo get_category_link($sub_sub_cat->term_id); ?>"><?php echo $sub_sub_cat->name; ?></a><span class="post_count"><?php echo " ($sub_sub_count+$sub_sub_count_prob)"; ?></span>
                                </li>
                            <?php endforeach; ?>
                            </ul>       
                        <?php endif; ?>
                    </li>       
                <?php endforeach; ?>
                </ul>   
            <?php endif; ?>
            </li>
        <?php endforeach; ?>
    </ul>
    <?php endif; ?>
</aside>
  • There's quite a bit that needs improved, luckily this code is readable though. Can you post the HTML output so any changes can be made sure to match what you have? Some things you should note, is to not open and close PHP so often. Right inside your <ul> you have 4 PHP statements. You could keep the first <?php before $cat_posts and the last ?> after count( $cat_probs ); and it would be better. Also, use WP_Query instead of get_posts. You can research why, but it's STRONGLY recommended to change. Also I think you could combine most of these category functions into the WP_Query – Xhynk Feb 27 '18 at 5:20
  • I thought I have to put <?php in every line of php code... ;( I am now deleting all redundant codes... Thanks! – jjycjn Feb 27 '18 at 5:36
  • Nope! Take a look at my answer below. All the <?php does is that it tells the document that PHP code will follow until the next ?>. Also look at this past bin, both of these blocks of code (ripped from yours) are functionally equivalent: pastebin.com/Q7wrHrTB – Xhynk Feb 27 '18 at 5:52
0

Original Code Sample Removed, updated based on new answer:

Honestly this doesn't change too much, other than makes it a bit more succinct, and I'll explain the major points of what I've done.

You reuse this code sample a lot:

<li class="cat-item"><a href="<?php echo get_category_link($cat->term_id); ?>"><?php echo $cat->name; ?></a><span class="post_count"><?php echo " ($cat_posts->found_posts+$cat_probs->found_posts)"; ?></span>

Inline, this gets relatively hard to read, and if you need to change or add a class to this, it gets a bit more tedious. Since it's repeated three times, you should probably turn it into a function, so at the start I've created a function called jjycjn_display_link() that takes the category object, as well as the objects returned by your posts query and probs query, and echo the output automatically, so now all you need to do is use:

<?php display_link( $cat, $cat_posts, $cat_probs ); ?>

Which is much easier to digest when reading a file like this.

Another thing you can do is assign variables inside an if statement. You don't need to assign a variable and then check if it exists, you can do that in one statement. So your

$categories = get_categories( array('parent' => 0, 'hide_empty' => 0, 'orderby' => 'term_order' ) );
if( $categories ):

is functionally equivalent to

if( $categories = get_categories( array('parent' => 0, 'hide_empty' => 0, 'orderby' => 'term_order' ) ) ):

That will assign $categories to the return value of get_categories(), and if it returns a Truthy Value then it will continue into the statement.

You might even be able to skip assigning the variables like $cat_posts and just drop the WP_Query into the jjycjyn_display_link() function since you don't use the assigned variable, but I've left it for readability and debugging purposes (easier to debug if you have assigned it to a variable)

Best of luck!

<?php
    function jjycjn_display_link( $cat, $posts, $probs ){
        echo '<a href="'. get_category_link($cat->term_id) .'">'. $cat->name .'</a><span class="post_count"> ('. $posts->found_posts + $probs->found_posts .')</span>';
    }
?>
<aside class="widget widget_categories">
<h3 class="widget-title"><span class="widget-title-tab">Categories</span></h3>  
    <?php if( $categories = get_categories( array('parent' => 0, 'hide_empty' => 0, 'orderby' => 'term_order' ) ) ):
        echo '<ul>';
        foreach($categories as $cat):
            $cat_posts = new WP_Query( array( 'cat' => $cat->term_id, 'post_type' => 'post' ) );
            $cat_probs = new WP_Query( array( 'cat' => $cat->term_id, 'post_type' => 'probsoln' ) ); ?>
            <li class="cat-item"><?php jjycjn_display_link( $cat, $cat_posts, $cat_probs ); ?>
            <?php if( $sub_categories = get_categories( array('parent' => $cat->term_id, 'hide_empty' => 0, 'orderby' => 'term_order' ) ) ):
                echo '<ul class="children">';
                foreach($sub_categories as $sub_cat):
                    $sub_cat_posts = new WP_Query( array( 'cat' => $sub_cat->term_id, 'post_type' => 'post' ) );
                    $sub_cat_probs = new WP_Query( array( 'cat' => $sub_cat->term_id, 'post_type' => 'probsoln' ) ); ?>
                    <li class="cat-item"><?php jjycjn_display_link( $sub_cat, $sub_cat_posts, $sub_cat_probs ); ?>
                    <?php if( $sub_sub_categories = get_categories( array('parent' => $sub_cat->term_id, 'hide_empty' => 0, 'orderby' => 'term_order' ) ) ):
                        echo '<ul class="children">';
                        foreach ($sub_sub_categories as $sub_sub_cat):
                            $sub_sub_cat_posts = new WP_Query( array ( 'cat' => $sub_sub_cat->term_id, 'post_type' => 'post' ) );
                            $sub_sub_cat_probs = new WP_Query( array ( 'cat' => $sub_sub_cat->term_id, 'post_type' => 'probsoln' ) ); ?>
                            <li class="cat-item"><?php jjycjn_display_link( $sub_sub_cat, $sub_sub_cat_posts, $sub_sub_cat_probs ); ?>
                        <?php endforeach;
                        echo '</ul>';     
                        endif;
                    echo '</li>';     
                endforeach;
                echo '</ul>';   
                endif;
            echo '</li>';
        endforeach;
        echo '</ul>';
    endif; ?>
</aside>
  • Thanks for the answer. Could you elaborate a little bit more? Which part of my code should be replaced by yours? – jjycjn Feb 27 '18 at 5:32
  • Almost all of it. I'll elaborate a bit more with your code wrapping it. – Xhynk Feb 27 '18 at 5:35
  • I've updated the code, you should be able to basically swap it out entirely. It uses far fewer functions calls, a single PHP tag, doesn't rely on expensive get_post queries to tally, and should work all the way down to Grand Child categories. I may have a typo or two, but the idea should provide a much easier to maintain and much lighter weight solution than what you have now – Xhynk Feb 27 '18 at 5:43
  • Also, there very well may be a plugin that accomplishes this type of thing already. I don't know for certain, but it may be worth looking at and dissecting how they do it. The above code is basically just a clean-up of the code you provided – Xhynk Feb 27 '18 at 5:44
  • This is working almost as I wanted, but for some reason, it returns 0 for the number of parent and child category (it counts the number of post in grandchild category, though). Also, I need to count the numbers of normal posts and a custom posts (having slug probsoln) separately in each category. Do you have any suggestion to achieve this goal? – jjycjn Feb 27 '18 at 6:05
0

Thanks to @Xhynk, I have made a change in my previous messy code. get_posts is now changed to WP_Query and all unnecessary <?php and ?> pairs are removed. I have no idea if this one is better than the previous one, but it is certainly shorter! There should be a way to call WP_Query function only once in each for loop, but I couldn't find a way to do it. I will updated the answer once I get a solution.

<aside class="widget widget_categories">
<h3 class="widget-title"><span class="widget-title-tab">Categories</span></h3>  
    <?php $categories = get_categories( array('parent' => 0, 'hide_empty' => 0, 'orderby' => 'term_order' ) );
    if($categories):
    echo '<ul>';
    foreach($categories as $cat):
        $cat_posts = new WP_Query( array ( 'cat' => $cat->term_id, 'post_type' => 'post' ) );
        $cat_probs = new WP_Query( array ( 'cat' => $cat->term_id, 'post_type' => 'probsoln' ) ); ?>
        <li class="cat-item" style=""><a href="<?php echo get_category_link($cat->term_id); ?>"><?php echo $cat->name; ?></a><span class="post_count"><?php echo " ($cat_posts->found_posts+$cat_probs->found_posts)"; ?></span>
        <?php $sub_categories = get_categories( array('parent' => $cat->term_id, 'hide_empty' => 0, 'orderby' => 'term_order' ) );
            if($sub_categories):
            echo '<ul class="children">';
            foreach ($sub_categories as $sub_cat):
                $sub_cat_posts = new WP_Query( array ( 'cat' => $sub_cat->term_id, 'post_type' => 'post' ) );
                $sub_cat_probs = new WP_Query( array ( 'cat' => $sub_cat->term_id, 'post_type' => 'probsoln' ) ); ?>
                <li class="cat-item"><a href="<?php echo get_category_link($sub_cat->term_id); ?>"><?php echo $sub_cat->name; ?></a><span class="post_count"><?php echo " ($sub_cat_posts->found_posts+$sub_cat_probs->found_posts)"; ?></span>
                <?php $sub_sub_categories = get_categories( array('parent' => $sub_cat->term_id, 'hide_empty' => 0, 'orderby' => 'term_order' ) );
                    if($sub_sub_categories):
                    echo '<ul class="children">';
                    foreach ($sub_sub_categories as $sub_sub_cat):
                        $sub_sub_cat_posts = new WP_Query( array ( 'cat' => $sub_sub_cat->term_id, 'post_type' => 'post' ) );
                        $sub_sub_cat_probs = new WP_Query( array ( 'cat' => $sub_sub_cat->term_id, 'post_type' => 'probsoln' ) ); ?>    
                        <li class="cat-item"><a href="<?php echo get_category_link($sub_sub_cat->term_id); ?>"><?php echo $sub_sub_cat->name; ?></a><span class="post_count"><?php echo " ($sub_sub_cat_posts->found_posts+$sub_sub_cat_probs->found_posts)"; ?></span>
                        </li>
                    <?php endforeach;
                    echo '</ul>';       
                    endif;
                echo '</li>';       
            endforeach;
            echo '</ul>';   
            endif;
        echo '</li>';
    endforeach;
    echo '</ul>';
    endif; ?>
</aside>
  • Nicely done! I'm going to edit my answer to reflect a few changes in what you have, but that already looks much better! – Xhynk Feb 27 '18 at 16:36
  • As well, I'm not sure you can call WP_Query once in each loop. You can change 'post_type' => 'probs' to 'post_type' => array( 'post', 'probs' ) which would work, but then you'd have to loop through that count each post type. It might be better to just stick with the two native queries. You may also consider installing the Query Monitor plugin to make sure it's not getting too heavy – Xhynk Feb 27 '18 at 16:59
  • Lastly, once you get a little more adept at PHP, you may want to consider adding a new option that's an associate array of all the posts/probs using update_option() function that updates when a post is published or removed so you're running such an expensive query each page load. Until then, consider caching your results using something like WP Super Cache or PHPFastCache to limit queries if they're expensive! – Xhynk Feb 27 '18 at 17:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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