saltedm8 Posted March 27, 2017 Share Posted March 27, 2017 Hi I am trying to create a loop function so that whenever I add parameters to the $menu_items, it adds another menu item to the construct. I have been trying for a couple of days, I think I need some direction to help me, I am struggling, I am fairly new to php and really new to classes and oop.. thanks <?php class menu { public function __construct($menu){ echo '<li>' . $menu . '<li>'; } } $menu_items = new menu('one', 'two', 'three', 'four'); ?> Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 Your constructor has one argument being passed to it which is currently a comma separated string. The method simply takes that entire string and outputs it as one list element. Is that what you want here? As a constructor I'm thinking that you would probably pass an array into the constructor to "construct" a new instance of a 'menu' object. Then, you might have another method to write out the menu itself, using the array as the source of the list items. Just a guess but hopefully you see the difference here. One other point - if this is all your class is going to do, why not just output the elements in your calling code and skip the class? Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 No, sorry, I posted that in order to show the basic idea of the sorts of things I am trying. I have tried a loop and a foreach, but basically, I want a class where all I have to do is type $menu_items = new menu('one', 'two', 'three', 'four'); and it produces the amount of items i have stated there.. the only other thing my brain dead mind forgot was linking lol.. forgot to add those too. Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 I see no need for a class that simply does one thing. As for your attempts, as is said here quite often - Show Us the Code. Quote Link to comment Share on other sites More sharing options...
Psycho Posted March 27, 2017 Share Posted March 27, 2017 (edited) One specific problem I see in your code is how you are using the class. perhaps that is the source of your problem and you are not explaining it well. $menu_items = new menu('one', 'two', 'three', 'four'); You are setting $menu_items to the return value of the class instantiation, However, the constructor has no return value. Instead the constructor is outputting content to the page. You absolutely should have the class return the content instead of outputting it. To add to ginerjm's response, I don't see a need for a class either if it has only one purpose of creating the menu links. Perhaps just a function would do. <?php function createMenuLinks($menuAry) { $menuHTML; foreach($menuAry as $label => $url) { $menuHTML .= "<li><a href='{$url}'>{$label}</a></li>\n"; } return $menuHTML; } $menuItems = array( 'Home' => 'index.php', 'About Us' => 'about.php', 'Services' => 'services.php', 'Contact Us' => 'contact.php' ); $menuLinks = createMenuLinks($menuItems); ?> <ul class="menu"> <?php echo $menuLinks; ?> </ul> Edited March 27, 2017 by Psycho Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 (edited) One specific problem I see in your code is how you are using the class. perhaps that is the source of your problem and you are not explaining it well. $menu_items = new menu('one', 'two', 'three', 'four'); You are setting $menu_items to the return value of the class instantiation, However, the constructor has no return value. Instead the constructor is outputting content to the page. You absolutely should have the class return the content instead of outputting it. To add to ginerjm's response, I don't see a need for a class either if it has only one purpose of creating the menu links. Perhaps just a function would do. <?php function createMenuLinks($menuAry) { $menuHTML; foreach($menuAry as $label => $url) { $menuHTML .= "<li><a href='{$url}'>{$label}</a></li>\n"; } return $menuHTML; } $menuItems = array( 'Home' => 'index.php', 'About Us' => 'about.php', 'Services' => 'services.php', 'Contact Us' => 'contact.php' ); $menuLinks = createMenuLinks($menuItems); ?> <ul class="menu"> <?php echo $menuLinks; ?> </ul> Thanks, the reason for the class is simply one of practice, since I am only just learning classes, I am not using this in production, but the one way it could be used in production is a template where you could instantiate a class and place in the names of the links you want to put there directly without doing through the function as I am pretty sure this sort of issue will crop up when I know enough to build with it. Could not find anywhere that would cover this type of issue. <?php class navigation { public $menuitems; public function menu_items($menuitems){ for($i=0;$i<$menuitems;$i++){ echo $i; } } } $my_menu = new navigation(); $my_menu -> menu_items('one', 'two', 'three', 'four'); ?> produces nothing. Edited March 27, 2017 by saltedm8 Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 I'm gonna bet that you if you had enabled php error checking it would have shown you the error in your code. Your for loop is not written correctly. "count($menuitems)" See my signature Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 I'm gonna bet that you if you had enabled php error checking it would have shown you the error in your code. Your for loop is not written correctly. "count($menuitems)" See my signature Actually, I have been using phpfiddle, although I did add that and did not get any error backs (except a phpfiddle ini set) Quote Link to comment Share on other sites More sharing options...
maxxd Posted March 27, 2017 Share Posted March 27, 2017 (edited) Your class constructor is only set to accept one parameter right now. So by attempting to pass it 4, it's ignoring the last three and there's nothing to loop over. A better method would be to pass the object an array of links to begin with, then (building on what Psycho said) output the menu using a different method altogether. $itemsOne = [ 'one', 'two', 'three', 'four' ]; $itemsTwo = [ 'five', 'six', 'seven', 'eight' ]; $menu1 = new MenuMaker($itemsOne); $menu2 = new MenuMaker($itemsTwo); $menu1->printMenu(); $menu2->printMenu(); class MenuMaker{ /** * @var array Numerically indexed array of menu items */ private $_items; /** * Stores the passed-in array into the local property '_items' * @param array $items */ public function __construct(array $items){ $this->_items = $items; } /** * Outputs the menu as an unordered list * @return void */ public function printMenu(){ print('<ul class="menu">'); foreach($this->_items as $item){ print("<li>".htmlspecialchars($item)."</li>"); } print('</ul>'); } } If you're determined to do it the way you've been trying to, you'll have to use func_get_args(), like so: class navigation{ public function __construct(){ print('<ul class="menu">'); foreach(func_get_args() as $item){ print("<li>".htmlspecialchars($item)."</li>"); } print('</ul>'); } } Edited March 27, 2017 by maxxd Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 (edited) Got it! This is what I was trying to do, I really appreciate the help though it. thank you (the rest should be simple enough) <?php class listing{ public function list(){ $list = func_get_args(); foreach ($list as $lists) echo $lists; } } $listed = new listing(); $listed->list('one ','two ','three ','four '); echo '</br>'; $listed->list('five ','six ','seven ','eight '); ?> Edited March 27, 2017 by saltedm8 Quote Link to comment Share on other sites More sharing options...
Psycho Posted March 27, 2017 Share Posted March 27, 2017 Got it! This is what I was trying to do, I really appreciate the help though it. thank you (the rest should be simple enough) Since is mostly for learning, I want to guide you to using good practices. As I stated before, the class should not actually output content. The class should return the content. you can take that return value and assign it to a variable and/or output it to the page. Typically you want to separate your logic (PHP) from the output (HTML). Intermixing PHP & HTML makes it much harder to maintain your code. Later you will learn to completely separate processes into different files, but for now put all of your logic at the top of the page and the output at the bottom. Just output PHP variables in the output. Example <?php class listing { public function list() { $list = func_get_args(); foreach ($list as $lists) { echo $lists; } } } $listed = new listing(); $menuList = $listed->list('one ','two ','three ','four '); $menuList .= "<br>"; $menuList .= $listed->list('five ','six ','seven ','eight '); ?> <html> <body> <?php echo $menuList; ?> </body> </html> Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 Thank you, I will try to keep that in mind, since I am so new to this (took a very long time out of php) I am simply thrilled to get stuff working and obviously as a result might forget this at times, thanks again Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 (edited) When doing codeing it is helpful to use meaningful var names to make it easy to come back a year later and do maintenance or to introduce someone new to the code. Your statement $list = func_get_args(); foreach ($list as $lists) is confusing. $list is an array - multiple things. The usage of $lists in the foreach is to select just ONE item from the array. A better approach might be: $items = func_get_args(); // a plural tense foreach($items as $item) // a singular tense which makes a lot more sense to browse thru and quickly understand. Edited March 27, 2017 by ginerjm Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 So much to take in lol, thanks again Quote Link to comment Share on other sites More sharing options...
maxxd Posted March 27, 2017 Share Posted March 27, 2017 I just want to reiterate what both Psycho and ginerjm have pointed out - this isn't the way you should be doing it. I put that solution there hoping the "If you're determined to do it the way you've been trying to..." would dissuade you from continuing to try to do it this way. The first example is a much better solution. Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 You say that you are just learning classes. What are you using as a teaching resource? You have so many things wrong it's like you haven't done any understanding how to work with a class. Plus - again - IMHO one writes a class for something that will have several properties and processes/things associated with it and where one can combine them into the class definition and use that code multiple times. I really don't see how a menu fits into that unless you are writing a multi-screen application that will have many pages on which you want to display the same items. Although even then I would use a simple function as was offered up before. Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 A Udemy course called 'learn object orintated php by building a complete website' I have not finished it yet, but I have viewed both the basic and advanced sections It probably does not help that I am a designer and have been away from php for at least 8 years. Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 It would seem to be a course lacking in the basics. You completely missed the concept of what a constructor does and how it returns the handle to the just-instantiated object, instead of some output that you are generating. You also seem to be rather weak in the use of arguments in method/function headers, which is definitely something you should have under your belt before embarking on classes. Sorry to be so brusk but you are reaching ahead a bit too far I think. Quote Link to comment Share on other sites More sharing options...
saltedm8 Posted March 27, 2017 Author Share Posted March 27, 2017 (edited) Bit of an understatement lol, I'm weak on everything, but I'm trying with the resources I have to learn php, I get quite distracted, learning 3D techniques in cinema 4d, keeping up with design standards, adobe software and I have only just learned version control through git.. up until 3 months ago I knew nothing about scss for the example (went to uni as a mature student to get a degree in web/graphic design - 3 years out of this trade), and that's not counting that I am an avid gym goer and have an interest in political debate.. I am all over the place lol but, again, I am trying.. I am grateful for any guidance and help Edited March 27, 2017 by saltedm8 Quote Link to comment Share on other sites More sharing options...
ginerjm Posted March 27, 2017 Share Posted March 27, 2017 Again - IMHO - stick to learning php well. Learn how to write a good function, a good secure db connection module that you can use in all your scripts. Learn how to write a good login script using the latest security techniques and make sure all this works well. Focus on the basics such as how a function comes into play and how to write on. Read up on and use good security when processing user input and when writing queries. Learn how to use a prepared query! When you figure all this out then move on to more advanced stuff, such as OOP. PS - this func_get_args call is IMO not worth using. Specify the parmaters you want in a function header and then use them as the arguments for when you call the function. You don't want to come back a month from now and try to figure out what the function needs to have when you try to use it again. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.