HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Navigation bar using PHP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
phpnavigationusingbar

Problem

I would like to know what you think:

"><?php
            //this part is only for the (google chrome) view-source:\\
            //this will not be shown into the normal layout/output\\
            if($name != $last_normal){
                    echo("\n                                        ");
            }else{
                    echo("\n");
            }

}


On each page I write at the place of the navigation bar:



I hope you can give me a review on what you think is either good or bad. Just give your opinion.

Solution

";

foreach($nav_normal as $name){

    if($Nav_ID == $name){
        // $Nav_Type is not very descriptive
        // $Nav_Type = "Selected";
        $Nav_Selected = "Selected";
    } else {
        $Nav_Selected = '';
        // it is not necessary to have a state for unselected
//        $Nav_Type = "Unselected";
    }

    // the last if/else block could be replaced by this single line
    $Nav_Selected = ($Nav_ID == $name) ? "Selected" : '';

    // it is not worth escaping php for a single, line, it is easier just to embed the html and echo it
    // make sure we call htmlspecialchars or characters like >".htmlspecialchars($name)."";

    // this makes no visible difference, multiple spaces are rendered as one in html
//    if($name != $last_normal){
//        echo("\n                                        ");
//    }else{
        echo "\n";
//    }

}

// if we want extra spaces after the last item in the loop, just echo them after the loop has finished
echo "                                        ";

// don't forget to close the ordered list too
echo "";

Code Snippets

<?php

// first up i have removed the numbers, those numbers can be automatically added using an ordered list,
// then if you add a menu option later you don't have to renumber everything and look through every file to check the $Nav_ID matches
$nav_normal = array("Home","Read Me","License Agreement","General Information","Database Installer","Create an Account","Create Config","Successfully installed");

// removed as documented below
// $last_normal = array_pop(array_keys($nav_normal));

// we need to start the list somewhere?, using an ordered list will automatically number the menu options
echo "<ol>";

foreach($nav_normal as $name){

    if($Nav_ID == $name){
        // $Nav_Type is not very descriptive
        // $Nav_Type = "Selected";
        $Nav_Selected = "Selected";
    } else {
        $Nav_Selected = '';
        // it is not necessary to have a state for unselected
//        $Nav_Type = "Unselected";
    }

    // the last if/else block could be replaced by this single line
    $Nav_Selected = ($Nav_ID == $name) ? "Selected" : '';



    // it is not worth escaping php for a single, line, it is easier just to embed the html and echo it
    // make sure we call htmlspecialchars or characters like >< & " etc will stuff up your html
    echo "<li class=\"$Nav_Selected\">".htmlspecialchars($name)."</li>";

    // this makes no visible difference, multiple spaces are rendered as one in html
//    if($name != $last_normal){
//        echo("\n                                        ");
//    }else{
        echo "\n";
//    }

}

// if we want extra spaces after the last item in the loop, just echo them after the loop has finished
echo "                                        ";

// don't forget to close the ordered list too
echo "</ol>";

Context

StackExchange Code Review Q#49015, answer score: 3

Revisions (0)

No revisions yet.