Return to Answer

• 1

• 101
• 1

Here is my take on it...

<style type="text/css">
.section-title{
 font-weight: bold;
}
 
</style>
<?php 
 $h2_array['message'] = '<span class="section-title">h2 tags</span> - ';
 switch ($this->count_h2) {
 case 0:
 $h2_array['message'] .= 'No tags found. Add some!';
 $h2_array['status'] = 2;
 break;
 case '1':
 $h2_array['message'] .= 'Some found. Too few!';
 $h2_array['status'] = 1;
 break;
 default:
 $h2_array['message'] .= 'Many found. Great!';
 $h2_array['status'] = 0;
 break;
 }
 
 $h2_array['message'] .= '<span class="counter">' 
 . $this->count_h2 
 . '</span>';
 $h2_array['count'] = $this->count_h2; // Why?
 
 // BIG COMMENT SERVES AS A VISUAL DIVIDER OF SECTIONS
 
 $h3_h6_array['message'] = '<span class="section-title">h3-h6 tags</span> - ';
 
 $h3_h6_array['count'] = $this->count_h3 
 + $this->count_h4 
 + $this->count_h5 
 + $this->count_h6;
 // Realizing there are only two options,
 // this should be an if statement, not a switch.
 switch ($h3_h6_array['count']) {
 case 0:
 $h3_h6_array['message'] .= 'No found. Add some!';
 $h3_h6_array['status'] = 1;
 break;
 default:
 $h3_h6_array['message'] .= 'Found, great!';
 $h3_h6_array['status'] = 0;
 break;
 }
 if ($h3_h6_array['count']) 
 {
 $h3_h6_array['message'] .= '<span class="counter">' 
 . $h3_h6_array['count'] 
 . '</span>';
 }
?>

Now, I only did the first two sections, but I think you get the idea and can repeat it on the third. I echo other comments here about mixing presentation with logic, but I also recognize you are just getting into coding and sometimes practical application is more important.

##What I did:

• I cleaned up the arrays. Even with autocomplete in my IDE, typing all those array names was a nightmare. It doesn't have to be so complicated.

• I got rid of quite a few unnecessary variables. Message start and end just didn't make sense. Variables are values you want to reuse. But you were only using them one time. I also moved them so they were assigned in the order of the final output. This just gave it a more logical flow to me.

• I switched your strong tags to spans with classes.

A few techniques to note:

1. Comment your code. Don't be afraid to be verbose. Write what you were thinking when you wrote the code and why you make specific decision. That reference will help you tremendously down the road.

2. When you use an if statement, know that false, null, 0, empty strings and empty arrays will ALL will equate to false. So, instead of if($this->count_h2 == 0){ ... } you can just write if($this->count_h2){ ... }.

   Now, I don't know what your count_h2 method returns. Many methods/functions will return false or null when there is an error, in which case NEITHER of the above methods would work. If you want to make sure you are testing for the numeral zero and not for the boolean value false, you need to use three equal signs: if($this->count_h2 === 0){ ... }

3. When you echo all of this to the browser, it's going to look ugly and run together on a single line, making it difficult to read. Let me share with you a method I wish more php developers would use. It makes the output a bit prettier, without putting \n chars all through your code:

<?php
// Make an array, adding a new value for each line you want to print to the browser.
$message[] = '<span class="section-title">Title keywords</span> - ';
$message[] = 'No primary added.';
$message[] = '<span class="counter">' . $count . '</span>';
// Echo using implode, which joins each item in the array together to form a string. Each item is separated by a "new line".
echo implode("\n", $message);
?>

I hope that helps some. Good luck to you and keep learning. You will hit a point where it all gets much easier.