ansaurus

Question

Is there any better way then defining the function for this code in PHP?

Answer 1

+1 A:

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...

Robin 2010-09-06 20:37:39

thank you for making me aware of PHP Templating Engine, this is the first time i am seeing it, i guess it uses some kinda patterns used by cakephp.

Ibrahim Azhar Armar 2010-09-06 20:52:32

Answer 2

+1 A:

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!

mkoistinen 2010-09-06 20:41:29

i had the samthing in mind, that was the reason for asking the question here, i knew i was wrong ouputting zeros to all div's which is invalid, and a bad practice even though it works. i am going with this logic.

Ibrahim Azhar Armar 2010-09-06 20:44:14

i had to add another else condition there.

Ibrahim Azhar Armar 2010-09-06 21:07:22

Answer 3

+2 A:

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));

Kwebble 2010-09-06 21:15:13

that looks clean and neat. and the code is reduced like hell. thank you for that info.

Ibrahim Azhar Armar 2010-09-06 21:25:10

Answer 4

A:

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>

OIS 2010-09-06 22:06:52

ansaurus

tags:

views:

answers:

Is there any better way then defining the function for this code in PHP?

related questions