OO help

HDFilmMaker2112 · June 16, 2012

I'm trying to make a class for my menu generation. One generates a row of links, the other generates a drop down menu, but their cores are pretty much identical, so I figure I could condense these into a single class with two functions, a construct, and return function.

class menu{
public function __construct($array1, $index1, $array2, $index2, $select = null){

	if(isset($index2) && $index2!=null){
		// logic to determine which menu to use
		if(isset($_SESSION['username']) && $_SESSION['username']!=null){ 
		$this->type = $index1; 
		$this->menu_array = $array1;
		} 
		else{
		$this->type = $index2; 
		$this->menu_array = $array2;
		}
	}
	else{
	$this->type = $index1;
	$this->menu_array = $array1;
	}
	//Get Text for Links from Keys of Array
	$menu_text = array_keys($this->menu_array[$this->type]);

	// produce and output the correct menu
	$i=0;
		if(isset($select)){
		$newmenu="<select>"; 
		}
		foreach(array_values($menu_array[$type]) as $link_option){
		$newmenu.=$this->menu;
		$i++;
		}
		if(isset($select)){
		$newmenu.="</select>"; 
		}
}

public function generateTopMenu(){
$this->menu='<a href="'.$link_option.'">';
$this->menu.=$menu_text[$i];
$this->menu.='</a> ';
}

public function generateSearchMenu(){
$this->menu.='<option value="'.$search_text[$i].'" >'.$search_item.'</option>'."\n";
}

public function returnMenu(){
return $newmenu;
}

//Generate menu array
$menu_array = array();
$menu_array['loggedin'] = array("Home" => "/home","News Feed" => "/newsfeed","Notifications" => "/notifications","Requests" => "/requests", "Messages" => "/messages");
$menu_array['loggedout'] = array("Music" => "/music","Movies" => "/movies","Television" => "/television","Games" => "/games", "Videos" => "/videos", "Browse People" => "/people");


$toplinks = new menu($menu_array['loggedin'], "loggedin", $menu_array['loggedout'], "loggedout");
$toplinks->generateTopMenu();
echo $toplinks->returnMenu();

I'm sure there's a way to make this class simpler, I just don't know it.

This is currently throwing the following error notices:

Warning: Illegal offset type in /homepages/27/d418565624/htdocs/class.php on line 118

Warning: array_keys() expects parameter 1 to be array, string given in /homepages/27/d418565624/htdocs/class.php on line 118

Warning: array_values() expects parameter 1 to be array, null given in /homepages/27/d418565624/htdocs/class.php on line 125

Warning: Invalid argument supplied for foreach() in /homepages/27/d418565624/htdocs/class.php on line 125

Ad

HDFilmMaker2112 · June 16, 2012

Okay, well I got it to the following. It doesn't show any error notices, but it also doesn't display anything.

class menu{
public function __construct($array1, $index1, $array2, $index2, $select = null){

	if(isset($index2) && $index2!=null){
		// logic to determine which menu to use
		if(isset($_SESSION['username']) && $_SESSION['username']!=null){ 
		$this->menu_array[$this->index1] = $array1;
		} 
		else{
		$this->menu_array[$this->index2] = $array2;
		}
	}
	else{
	$this->menu_array[$this->index1] = $array1;
	}
	//Get Text for Links from Keys of Array
	$menu_text = array_keys($this->menu_array);

	// produce and output the correct menu
	$i=0;
		if(isset($select)){
		$newmenu="<select>"; 
		}
		foreach(array_values($this->menu_array) as $link_option){
		$newmenu.=$this->menu;
		$i++;
		}
		if(isset($select)){
		$newmenu.="</select>"; 
		}
}

public function generateTopMenu(){
$this->menu='<a href="'.$link_option.'">';
$this->menu.=$menu_text[$i];
$this->menu.='</a> ';
}

public function generateSearchMenu(){
$this->menu.='<option value="'.$search_text[$i].'" >'.$search_item.'</option>'."\n";
}

public function returnMenu(){
return $newmenu;
}


}


//Generate menu array
$menu_array = array();
$menu_array['loggedin'] = array("Home" => "/home","News Feed" => "/newsfeed","Notifications" => "/notifications","Requests" => "/requests", "Messages" => "/messages");
$menu_array['loggedout'] = array("Music" => "/music","Movies" => "/movies","Television" => "/television","Games" => "/games", "Videos" => "/videos", "Browse People" => "/people");


$toplinks = new menu($menu_array['loggedin'], "loggedin", $menu_array['loggedout'], "loggedout");
$toplinks->generateTopMenu();
echo $toplinks->returnMenu();

HDFilmMaker2112 · June 16, 2012

Just to bring things up to date again, I'm now trying this:

class menu{
public function __construct($menu_array, $index, $select = null){

	//Get Text for Links from Keys of Array
	$menu_text = array_keys($this->menu_array);

	// produce and output the correct menu
	$i=0;
		if(isset($select) && $select!=null){
		$this->newmenu="<select>"; 
		}
		foreach(array_values($this->menu_array) as $link_option){
		$this->newmenu.=$this->menu;
		$i++;
		}
		if(isset($select) && $select!=null){
		$this->newmenu.="</select>"; 
		}
}

public function generateTopMenu(){
$this->menu='<a href="'.$link_option.'">';
$this->menu.=$menu_text[$i];
$this->menu.='</a> ';
}

public function generateSearchMenu(){
$this->menu.='<option value="'.$search_text[$i].'" >'.$search_item.'</option>'."\n";
}

public function returnMenu(){
return $this->newmenu;
}


}

//Generate menu array
$menu_array = array();
$menu_array['loggedin'] = array("Home" => "/home","News Feed" => "/newsfeed","Notifications" => "/notifications","Requests" => "/requests", "Messages" => "/messages");
$menu_array['loggedout'] = array("Music" => "/music","Movies" => "/movies","Television" => "/television","Games" => "/games", "Videos" => "/videos", "Browse People" => "/people");

// logic to determine which menu to use
if(isset($_SESSION['username'])){ $type = 'loggedin'; } 
else { $type = 'loggedout'; }

$toplinks = new menu($menu_array[$type], $type);
$toplinks->generateTopMenu();
echo $toplinks->returnMenu();

Error notices are back:

Warning: array_keys() expects parameter 1 to be array, null given in /homepages/27/d418565624/htdocs/class.php on line 103

Warning: array_values() expects parameter 1 to be array, null given in /homepages/27/d418565624/htdocs/class.php on line 110

Warning: Invalid argument supplied for foreach() in /homepages/27/d418565624/htdocs/class.php on line 110

Seems as though the array isn't being passed as an array.

trq · June 16, 2012

Seems as though the array isn't being passed as an array.

You never set $this->menu_array to anything.

smoseley · June 16, 2012



class Menu {
private $newmenu;
private $menu;
public function __construct($menu_array, $index, $select = null){

	//Get Text for Links from Keys of Array
	$menu_text = array_keys($menu_array);

	// produce and output the correct menu
	$i=0;
	if(isset($select) && $select!=null){
		$this->newmenu="<select>"; 
	}
	foreach(array_values($this->menu_array) as $link_option){
		$this->newmenu.=$this->menu;
		$i++;
	}
	if(isset($select) && $select!=null){
		$this->newmenu.="</select>"; 
	}
}

public function generateTopMenu(){
	$this->menu='<a href="'.$link_option.'">';
	$this->menu.=$menu_text[$i];
	$this->menu.='</a> ';
}

public function generateSearchMenu(){
	$this->menu.='<option value="'.$search_text[$i].'" >'.$search_item.'</option>'."\n";
}

public function returnMenu(){
	return $this->newmenu;
}
}

trq · June 16, 2012



class Menu {
private $newmenu;
private $menu;
public function __construct($menu_array, $index, $select = null){

	//Get Text for Links from Keys of Array
	$menu_text = array_keys($menu_array);

	// produce and output the correct menu
	$i=0;
	if(isset($select) && $select!=null){
		$this->newmenu="<select>"; 
	}
	foreach(array_values($this->menu_array) as $link_option){
		$this->newmenu.=$this->menu;
		$i++;
	}
	if(isset($select) && $select!=null){
		$this->newmenu.="</select>"; 
	}
}

public function generateTopMenu(){
	$this->menu='<a href="'.$link_option.'">';
	$this->menu.=$menu_text[$i];
	$this->menu.='</a> ';
}

public function generateSearchMenu(){
	$this->menu.='<option value="'.$search_text[$i].'" >'.$search_item.'</option>'."\n";
}

public function returnMenu(){
	return $this->newmenu;
}
}

Nice try smoseley, but $this->menu_array is still undefined.

HDFilmMaker2112 · June 16, 2012

Still not displaying anything. I used print_r on $this->menu_array and it did print out the array, it's just not running it through the foreach statement.

class menu{

        private $newmenu;
        private $menu;
public function __construct($array=array(), $index, $select = null){

$this->menu_array=$array;

	//Get Text for Links from Keys of Array
	$menu_text = array_keys($this->menu_array);

	// produce and output the correct menu
	$i=0;
		if(isset($select) && $select!=null){
		$this->newmenu="<select>"; 
		}
		foreach(array_values($this->menu_array) as $link_option){
		$this->newmenu.=$this->menu;
		$i++;
		}
		if(isset($select) && $select!=null){
		$this->newmenu.="</select>"; 
		}
}

public function generateTopMenu(){
$this->menu='<a href="'.$link_option.'">';
$this->menu.=$menu_text[$i];
$this->menu.='</a> ';
}

public function generateSearchMenu(){
$this->menu.='<option value="'.$link_option[$i].'" >'.$menu_text.'</option>'."\n";
}

public function returnMenu(){
return $this->newmenu;
}
}

KevinM1 · June 16, 2012

Your code is very scattered, and pretty much gibberish.

1. You never declare $menu_array before using it.

2. You have $i for no reason, and don't do anything with it after incrementing it.

3. You use array_values for no reason, and never use $link_option in that loop (your other methods won't be able to 'see'/use $link_option due to scope).

4. You try appending $this->menu to $this->newmenu when $this->menu has no value.

---

Right now, it looks like you're panicking because the code isn't working right, and you're making random incremental changes hoping that somehow it'll fix the problem(s). Your first three posts - written in the span of 30 minutes - highlight this. You need to stop, take a deep breath, and really run through what you've already written. None of it currently makes any kind of sense. Do it with pen and paper, and map out the process. Then, compare what you want to happen with what your current code is doing.

HDFilmMaker2112 · June 16, 2012

Your code is very scattered, and pretty much gibberish.

1. You never declare $menu_array before using it.

2. You have $i for no reason, and don't do anything with it after incrementing it.

3. You use array_values for no reason, and never use $link_option in that loop (your other methods won't be able to 'see'/use $link_option due to scope).

4. You try appending $this->menu to $this->newmenu when $this->menu has no value.

---

Right now, it looks like you're panicking because the code isn't working right, and you're making random incremental changes hoping that somehow it'll fix the problem(s). Your first three posts - written in the span of 30 minutes - highlight this. You need to stop, take a deep breath, and really run through what you've already written. None of it currently makes any kind of sense. Do it with pen and paper, and map out the process. Then, compare what you want to happen with what your current code is doing.

What I'm trying to do is keep a majority of the code that is the same in __construct, then pull in the formatting/display structuring via generateTopMenu and generateSearchMenu, through $this->menu, then have the foreach statement and $i increment through the array pulled in via the __construct, applying the result (on $this->menu) to $this->new_menu, then return via the returnMenu function.

I guess I can't do it that way? Otherwise if it can't work that way, then I have no point in doing it.

KevinM1 · June 16, 2012

That sounds incredibly convoluted, and really not thinking in OO terms.

Your menu class needs to represent a menu. It shouldn't necessarily represent all kinds of menus, or every menu on your site. A menu needs to be a discrete thing, in and of itself. What a particular menu represents can be implemented in a variety of ways. You could pass in options through its constructor. You could subclass.

The point is, you're trying to jam all of your site's particular menu functionality into one object, instead of thinking in more abstract terms. OO is all about abstraction. That's one of the reasons why it's a popular paradigm. If you can abstract your code, and write code that interacts via interfaces (meaning the public members of other classes, and not necessarily the interface language construct), you'll have modular code.

What you're currently doing is the classic mistake of jamming code into classes because of some nebulous idea that "it's how the pro's do it." Without actually understanding the how or why, you're simply making everything harder for yourself.

I understand that your finances are tight, but you should really try to save ~$30 and get Zandstra's book. You're not going to be able to get a job at a development studio with your current quality of code, and for freelance projects, editing/debugging/maintenance would be a nightmare with what you have. This is really one of those "spend money to make money" moments. Far more efficient than writing some code and seeing if we can make heads or tails of it.

HDFilmMaker2112 · June 16, 2012

I just went with a function:

Works with no issues.

function menu($menu_array, $select = null){
	$array=$menu_array;
//Get Text for Links from Keys of Array
	$menu_text = array_keys($array);

	// produce and output the correct menu
	$i=0;
	$newmenu="";
		if(isset($select) && $select!=null){
		$newmenu.="<select>";
		$menu_type="select";
		}
		else{
		$menu_type="links";
		}
		foreach(array_values($array) as $link_option){
			if($menu_type=="links"){
			$newmenu.='<a href="'.$link_option.'">';
			$newmenu.=$menu_text[$i];
			$newmenu.='</a> ';
			}
			elseif($menu_type=="select"){
			$newmenu.='<option value="'.$link_option.'" >'.$menu_text[$i].'</option>'."\n";		
			}
		$i++;
		}
		if(isset($select) && $select!=null){
		$newmenu.="</select>";
		}
return $newmenu;
}

smoseley · June 16, 2012

Nice try smoseley, but $this->menu_array is still undefined.

Good point. But it's not supposed to be defined. He seems to want to reference $menu_array (param). I got one, guess I missed one.

Sign In

OO help

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Join the conversation

Important Information