Calling a method outside class statically, passing other method's output as parameter

Question

I have this class:

class adminTemplate { public static function load() { return new self(); } public function sidebarGuide() { return 'content'; // Very long html markup } public function wrapSidebars($content) { ob_start(); echo "<div class='sidebar wrapper'>"; echo $content; echo "</div>"; $output = ob_get_contents(); ob_end_clean(); return $output; } // Other methods here }

Somewhere in my script, I have to call wrapSidebars using sidebarGuide's output as parameter:

echo adminTemplate::load()->wrapSidebars(adminTemplate::load()->sidebarGuide());

It works, but I'm pretty confident there is a more appropriate way.

Is there a proper way of passing methods'output as other method's parameter in a class loaded "statically"?
Besides that, is there a more elegant code structure to accomplish the same "task"?

cbojar · Accepted Answer · 2014-08-19 21:31:52Z

This implementation confuses me a bit, and maybe it's because we aren't able to see the whole picture.

First, I don't understand the purpose of the adminTemplate::load() method. It is basically just a constructor. If you eventually plan to add more complexity to it (use it as a builder, factory, or to make adminTemplate a singleton), then it makes sense, but as it stands now, you might as well just use a constructor.

The relationship between sidebarGuide and wrapSidebars is not totally clear either. If wrapSidebars is only ever called with the result of sidebarGuide as a parameter, then drop the parameter and convert sidebarGuide into a private helper method. If, on the other hand, wrapSidebars can be called with more than the output of sidebarGuide, but sidebarGuide is always wrapped with wrapSidebars, simply wrap the return value of sidebarGuide:

public function sidebarGuide() { return $this->wrapSidebars('content'); // Very long html markup }

If sidebars are not always wrapped, but only sidebars are wrapped, you could create a decorator class to wrap around your adminTemplate instance that gets the content from the delegate, and returns the content, wrapped.

class adminTemplate { # Constructor function sidebarGuide() { ... } } class TemplateDecorator { protected $template; # Constructor protected function wrapSidebars() { ... } function sidebarGuide() { return $this->wrapSidebars($this->template->sidebarGuide()); } }

The one other thing that confuses me is the implementation of wrapSidebars. You use ob_* functions and echos, but all of the pieces are already strings? You could more simply use string concatenation/interpolation:

public function wrapSidebars($content) { return "<div class='sidebar wrapper'>$content</div>"; }

There may be a (very) minor difference in performance, but the simplicity of the code should outweigh the micro-optimization.

jsanc623 · Accepted Answer · 2014-08-19 20:00:23Z

You are calling load() every time you call either sidebarGuide or wrapSidebars. Are you doing anything other than returning an instance of the class? If not, then just instantiate the class and making load() a construct would be more ideal. Doing so, your class would look like:

class adminTemplate { public function __construct(){} public function sidebarGuide(){ return "content"; # Very long html markup } public function wrapSidebars($content){ ob_start(); echo "<div class='sidebar wrapper'>"; echo $content; echo "</div>"; $output = ob_get_contents(); ob_end_clean(); return $output; } }

You would then use it as follows:

$adminTemplate = new adminTemplate(); $adminTemplate->sidebarGuide(); $adminTemplate->wrapSidebars($adminTemplate->sidebarGuide());

If you wanted, you could even store the return value of sidebarGuide() in a property of the class so that you could call wrapSidebars() without injecting sidebarGuide(), as follows:

class adminTemplate { private $sidebarGuideReturnValue public function __construct(){} public function sidebarGuide(){ $this->sidebarGuideReturnValue = "content"; # Very long html markup return $this->sidebarGuideReturnValue; } public function wrapSidebars($content = ""){ # Make $content an optional parameter. If its not set, grab the property $content = ($content ? $content : $this->sidebarGuideReturnValue); ob_start(); echo "<div class='sidebar wrapper'>"; echo $content; echo "</div>"; $output = ob_get_contents(); ob_end_clean(); return $output; } }

If you know that would only be calling sidebarGuide() exclusively as a param to wrapSidebars(), you could even relegate sidebarGuide()'s to a private called on instantiation in the construct, or move its body to the construct itself.

One question that bothers me though is, is there any specific reason you want to use static load()? Or just because it looks nice?

You probably should not have answered this question while it contained example code. The OP has just now changed the names, so you may update your answer accordingly (it doesn't count as invalidation since the edit was in response to the closure). — Jamal
– Jamal, Commented Aug 19, 2014 at 18:35
Since I don't use autoloader (yet), the static load() allows me to bypass class instantiation, which I dislike. Storing a method output in a class poperty with the constructor is interesting, but will this lead a sidebarGuide call for every class instance, even when wrapSidebars is not needed? — Jolly Roger
– Jolly Roger, Commented Aug 19, 2014 at 19:40
@Jamal True. I had posted the answer on the page prior to the closing (only noticed closing after reloading the page). Will update to reflect new code. @Jolly Roger - Using an autoloader helps in loading the class, not in instantiating the class (read, you still have to instantiate after a basic autoload). Also, it's not about what you like/dislike, its about best practice...and calling load() every time only adds overhead and makes your code run slower (you're creating a new object every call). Also, yes it'll create sidebarGuide for the instance. — jsanc623
– jsanc623, Commented Aug 19, 2014 at 19:58
@jsanc623 Fair enough, you're right. And speaking about best practice, what about making methods static? So: adminTemplate::wrapSidebars(adminTemplate::sidebarGuide());? — Jolly Roger
– Jolly Roger, Commented Aug 19, 2014 at 20:06
You could do that - but then you'd lose state (remember a static method is an immutable method, then the drawback is that mutating operations need to copy an object instead of changing it in-place. That can get really expensive, really quickly). Perhaps read up on the Facade Design Pattern (sourcemaking.com/design_patterns/facade/php)? — jsanc623
– jsanc623, Commented Aug 19, 2014 at 20:33

Travis Weston · Accepted Answer · 2014-08-19 20:41:35Z

I like @jsanc623's answer, but I would also add into the code a private static variable $_instance, and using a similar loader function:

class adminTemplate { private static $_instance; private $sidebarGuideReturnValue private function __construct(){} public static function getInstance() { if(!(self::$_instance instanceof adminTemplate)) self::$_instance = new adminTemplate(); return self::$_instance; } public function sidebarGuide(){ $this->sidebarGuideReturnValue = "content"; # Very long html markup return $this->sidebarGuideReturnValue; } public function wrapSidebars($content = ""){ # Make $content an optional parameter. If its not set, grab the property $content = ($content ? $content : $this->sidebarGuideReturnValue); ob_start(); echo "<div class='sidebar wrapper'>"; echo $content; echo "</div>"; $output = ob_get_contents(); ob_end_clean(); return $output; } }

From there, you'll be able to effortlessly call adminTemplate::getInstance()->wrapSidebars();

Stack Exchange Network

Calling a method outside class statically, passing other method's output as parameter

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Calling a method outside class statically, passing other method's output as parameter

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions