current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I'm creating a PHP website for a non-profit. They have some restrictions (no MySQL or pre-installed CMS) so I'm creating a CSS menu displayed by an unordered list where all of the elements are stored in a multi-dimensional array.

I've gotten it to work but being new to PHP, I'm certain it's not optimized to run as smoothly as possible. Also, I feel I've hacked my way through the first and last elements.

Here's the array:

<?php
$siteURL = "http://www.mysite.org/";

$menu = array
   (
   //"Name","URL",ID,Level,Seq,Under,Target,Display
   //First Menu
   array("TOP1",$siteURL."top1.php",1,1,1,0,0,true),
   array("SUB1a",$siteURL."sub1a.php",2,2,1,1,0,true),
   array("SUB1b",$siteURL."sub1b.php",3,2,2,1,0,true),
   array("SUB1c",$siteURL."sub1c.php",4,2,3,1,0,true),
   array("SUB1d","http://www.externalsite.org/",5,2,4,1,1,true),
   array("SUB1e","http://www.externalsite.org/",6,2,5,1,1,true),
   array("SUB1f",$siteURL."sub1f.php",7,2,6,1,0,true),

   //Second Menu
   array("TOP2",$siteURL."top2.php",8,1,2,0,0,true),
   array("SUB2a","http://www.externalsite.org/",9,2,1,8,1,true),
   array("SUB2b",$siteURL."sub2b.php",10,2,2,8,0,true),
   array("SUB2c","http://www.externalsite.org/",11,2,3,8,1,true),
   array("SUB2d","http://www.externalsite.org/",12,2,4,8,1,true),

   //Third Menu
   array("TOP3",$siteURL."top3.php",13,1,3,0,0,true),
   array("SUB3a","http://www.externalsite.org/",14,2,1,13,1,true),
   array("SUB3b","http://www.externalsite.org/",15,2,2,13,1,true),
   array("SUB3c","http://www.externalsite.org/",16,2,3,13,1,true),
   array("SUB3d","http://www.externalsite.org/",17,2,4,13,1,true),

   //Fourth Menu
   array("TOP4",$siteURL."top4.php",18,1,4,0,0,true),
   array("SUB4a",$siteURL."downloads/sub4a.pdf",19,2,1,18,0,true),
   array("SUB4b",$siteURL."sub4b.php",20,2,2,18,0,true),

   //Fifth Menu
   array("TOP5",$siteURL."top5.php",21,1,5,0,0,true),

   //Sixth Menu
   array("TOP6",$siteURL."top6.php",22,1,6,0,0,false),

   //Seventh Menu
   array("TOP7",$siteURL."top7.php",23,1,7,0,0,false),

   //Eighth Menu
   array("TOP8","http://www.externalsite.org/",24,1,8,0,1,true),

   );
?>

And here's the logic:

<ul>
<?php
$count = count($menu);
$i = 0;
while ($i < $count) {
    if (($menu[$i][3]===1) && ($menu[$i][7]===true)) {
        if ($menu[$i][6]===1) {
            echo "<li><a href=\"".$menu[$i][1]."\" target=\"_blank\">".$menu[$i][0]."</a><ul>\n";
        }
        else {
            echo "<li><a href=\"".$menu[$i][1]."\">".$menu[$i][0]."</a><ul>\n";
        }
        for ($j = 0; $j < $count; $j++){
            if (($menu[$j][5]===$menu[$i][2]) && ($menu[$j][7]===true)) {
                if ($menu[$j][6]===1) {
                    echo "<li><a href=\"".$menu[$j][1]."\" target=\"_blank\">".$menu[$j][0]."</a></li>\n";
                }
                else {
                    echo "<li><a href=\"".$menu[$j][1]."\">".$menu[$j][0]."</a></li>\n";
                }
            }
        }
        echo '</ul></li>';
    }
    $i++;
}
?>
</ul>

I had a hard time finding a relevant post. I'd appreciate some help or even just validation that I'm doing this correctly.

share|improve this question
    
Welcome to Code Review! Just wanted to tell you that it's a nice little first-post you have here. It seems like you have understood exactly what this site is about. –  Simon Forsberg♦ Feb 23 '14 at 0:32

1 Answer 1

up vote 2 down vote

I'm not sure for optimization but for readability and maintenance I would use a Class and create the menu items as objects instead of multi-dimension arrays.

class MenuItem {

  protected $id;
  protected $name;
  protected $url;
  protected $level;
  protected $seq;
  protected $under;
  protected $target;
  protected $display;

  public function __construct($name, $url, $id, $level, $seq, $under, $target, $display) {
    $this->id = $id;
    $this->name = $name;
    $this->url = $url;
    $this->level = $level;
    $this->seq = $seq;
    $this->under = $under;
    $this->target = $target;
    $this->display = $display;
  }

  public function getId() {
    return $this->id;
  }

  public function getName() {
    return $this->name;
  }

  public function getUrl() {
    return $this->url;
  }

  public function getLevel() {
    return $this->level;
  }

  public function getSeq() {
    return $this->seq;
  }

  public function getUnder() {
    return $this->under;
  }

  public function getTarget() {
    return $this->target;
  }

  public function getDisplay() {
    return $this->display;
  }

}

And so your creation code would become:

$siteURL = "http://www.mysite.org/";

$menu = array
   (
   //"Name","URL",ID,Level,Seq,Under,Target,Display
   //First Menu
   new MenuItem("TOP1",$siteURL."top1.php",1,1,1,0,0,true),
   new MenuItem("SUB1a",$siteURL."sub1a.php",2,2,1,1,0,true),
   new MenuItem("SUB1b",$siteURL."sub1b.php",3,2,2,1,0,true),
   new MenuItem("SUB1c",$siteURL."sub1c.php",4,2,3,1,0,true),
   new MenuItem("SUB1d","http://www.externalsite.org/",5,2,4,1,1,true),
   new MenuItem("SUB1e","http://www.externalsite.org/",6,2,5,1,1,true),
   new MenuItem("SUB1f",$siteURL."sub1f.php",7,2,6,1,0,true),

   //Second Menu
   new MenuItem("TOP2",$siteURL."top2.php",8,1,2,0,0,true),
   new MenuItem("SUB2a","http://www.externalsite.org/",9,2,1,8,1,true),
   new MenuItem("SUB2b",$siteURL."sub2b.php",10,2,2,8,0,true),
   new MenuItem("SUB2c","http://www.externalsite.org/",11,2,3,8,1,true),
   new MenuItem("SUB2d","http://www.externalsite.org/",12,2,4,8,1,true),

   //Third Menu
   new MenuItem("TOP3",$siteURL."top3.php",13,1,3,0,0,true),
   new MenuItem("SUB3a","http://www.externalsite.org/",14,2,1,13,1,true),
   new MenuItem("SUB3b","http://www.externalsite.org/",15,2,2,13,1,true),
   new MenuItem("SUB3c","http://www.externalsite.org/",16,2,3,13,1,true),
   new MenuItem("SUB3d","http://www.externalsite.org/",17,2,4,13,1,true),

   //Fourth Menu
   new MenuItem("TOP4",$siteURL."top4.php",18,1,4,0,0,true),
   new MenuItem("SUB4a",$siteURL."downloads/sub4a.pdf",19,2,1,18,0,true),
   new MenuItem("SUB4b",$siteURL."sub4b.php",20,2,2,18,0,true),

   //Fifth Menu
   new MenuItem("TOP5",$siteURL."top5.php",21,1,5,0,0,true),

   //Sixth Menu
   new MenuItem("TOP6",$siteURL."top6.php",22,1,6,0,0,false),

   //Seventh Menu
   new MenuItem("TOP7",$siteURL."top7.php",23,1,7,0,0,false),

   //Eighth Menu
   new MenuItem("TOP8","http://www.externalsite.org/",24,1,8,0,1,true),

   );

Then you can access values in a more readable way. E.g.

foreach ($menu as $menuItem) {
  if (($menuItem->getLevel() === 1) && ($menuItem->getDisplay() === true)) {
    ...
  }
}
share|improve this answer
    
You could extend MenuItem by adding a subMenu property - which is of type MenuItem - and instead of creating all your MenuItems in an array, add the sub menus to the parent. –  Carl Porter Feb 23 '14 at 1:02
    
Thanks! This works great for the the top-level menu items but I'm lost when it comes to the loop for the sub-menu items. –  alp42 Feb 25 '14 at 14:26

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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