Is there any better way then defining the function for this code in PHP?_问答_开发者

I have an HTML menu and i have styled it with CSS to highlight the current page. as i have more then 8 pages using the same code. i wanted to do something dynamic and i created a PHP function for that,

here is my code..

 <?php    function hiddenmenu($activeicon1 = 0, $activeicon2 = 0, $activeicon3 = 0, $activeicon4 = 0,
                               $act开发者_运维百科iveicon5 = 0, $activeicon6 = 0, $activeicon7 = 0, $activeicon8 = 0, $activeicon9 = 0) {
 ?>

                    <div class="icon-no-spacer">
                        <p><a id="<?php echo $activeicon1; ?>" href="index.php" title=""><img src="images/icons/48/display.png" alt="" />Dashboard</a></p>
                    </div>

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon2; ?>" href="post-news.php" title=""><img src="images/icons/48/text_rtf.png" alt="" />Post News</a></p>
                    </div>

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon3; ?>" href="news.php" title=""><img src="images/icons/48/wordprocessing.png" alt="" />News</a></p>
                    </div> 

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon4; ?>" href="post-advertisement.php" title="" ><img src="images/icons/48/view_pim_news.png" alt="" />Post Ad</a></p>
                    </div>

                    <div class="icon-spacer">
                       <p><a id="<?php echo $activeicon5; ?>" href="advertisement.php" title=""><img src="images/icons/48/view_pim_tasks.png" alt="" />Advertise</a></p>
                    </div>

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon6; ?>" href="comments.html" title=""><img src="images/icons/48/spread.png" alt="" />Comments</a></p>
                    </div>

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon7; ?>" href="#" title=""><img src="images/icons/48/rss_tag.png" alt="" />Sponsors</a></p>
                    </div>

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon8; ?>" href="#" title=""><img src="images/icons/48/fileview_preview.png" alt="" />Video</a></p>
                    </div>

                    <div class="icon-spacer">
                        <p><a id="<?php echo $activeicon9; ?>" href="information.php" title="" ><img src="images/icons/48/help_about.png" alt="" />Information</a></p>
                    </div> 
<?php                  
}              
?>

the code is working perfectly fine. is it ok to go with this. ?? lot of people might have come accross this type of issue i want to know the best feasible solution for this..

thank you

I would do at least these 2 things differently:

separate the definition of the menu items from the rendering, for easier maintenance.
only render available choices to avoid structural errors.

Something like this can do that:

<?php
$choices = array(
    1 => array("index.php",              "display.png",          "Dashboard"),
    2 => array("post-news.php",          "text_rtf.png",         "Post News"),
    3 => array("news.php",               "wordprocessing.png",   "News"),
    4 => array("post-advertisement.php", "view_pim_news.png",    "Post Ad"),
    5 => array("advertisement.php",      "view_pim_tasks.png",   "Advertise"),
    6 => array("comments.html",          "spread.png",           "Comments"),
    7 => array("#",                      "rss_tag.png",          "Sponsors"),
    8 => array("#",                      "fileview_preview.png", "Video"),
    9 => array("information.php",        "help_about.png",       "Information")
);

function hiddenmenu($iconIds) {
    global $choices;

    foreach ($iconIds as $iconId){
        echo '<div class="icon-no-spacer">'
           . '<p><a id="' . $iconId . '"'
           . ' href="' . $choices[$iconId][0] . '"'
           . ' title=""><img src="images/icons/48/' . $choices[$iconId][1] . '"'
           . ' alt="" />' . $choices[$iconId][2] . '</a></p>' ."\n";
    }
}

hiddenmenu(array(1,2,3,8,9));

It's considered bad practice to mix up code and markup like that, what if a non-programmer had to change your code, would (s)he break something? A better idea is to use a templating engine, this way you have 2 files - a PHP script (no markup) that reads in a template file (markup + instructions) and uses them together to output HTML (+ dynamic content). Google for PHP Templating engine, there are billions to choose from...

I think what you want is something like this:

Note, I've abbreviated the code with ellipses (...) here and there.

<?php
   function hiddenmenu($activeicon1 = 0, $activeicon2 = 0, ..., $activeicon9 = 0) {
      if ($activeicon1) {
?>
         <div class="icon-no-spacer">
            <p><a id="activeicon1" href="index.php" title=""><img src="images/icons/48/display.png" alt="" />Dashboard</a></p>
         </div>
<?php
      }
      if ($activeicon2) {
?>

         <div class="icon-no-spacer">
            <p><a id="activeicon2" href="post-news.php" title=""><img src="images/icons/48/text_rtf.png" alt="" />Post News</a></p>
         </div>
<?php
      }
      ...
   }
?>

I'm not sure what the parameters of the function would be set to, but most of the time, it may be generating a number of divs all with the same ID: 0, which is invalid since IDs must start with a letter AND, they must be unique. Also, you needed logic to turn these menu items on and off. The if statements I used above should help here.

Good luck!

Guess Id make something overly complicated...

php code:

<?php
function processTemplate($_file, $_vars)
{
    //todo: check if file exists
    extract($_vars);
    ob_start();
    include $_file;
    return ob_get_clean();
}

function processArray($template, $choices) {
    $output = '';
    foreach ($choices as $choice) {
        $output .= processTemplate($template, $choice);
    }
    return $output;
}

$choices = array(
    1 => array(
        'id' => '1',
        'link' => "index.php",
        'image' => "images/icons/48/display.png",
        'text' => "Dashboard",
    ),
    2 => array(
        'id' => '2',
        'link' => "post-news.php",
        'image' => "images/icons/48/text_rtf.png",
        'text' => "Post News",
    ),
    //etc
);

$ids = array_flip(array(1,2));
$chosen = array_intersect_key($choices, $ids);
echo processArray('menu.phtml', $chosen);

Template file: menu.phtml

<div class="icon-no-spacer">
    <p>
        <a id="<?php echo $id; ?>" href="<?php echo $link; ?>" title="">
            <img src="<?php echo $image; ?>" alt="" />
            <?php echo $text; ?>
        </a>
    </p>
</div>