Tuesday, May 3, 2011

PHP constructor executes before arguments for nested variables can be supplied.

First things first, here is a little snippet code to help explain my problem:

<?php
class foo {

    public $title;

    __construct{

        echo "<html>\n";
        echo "<head>\n";
        echo "<title>".$this->title."</title>\n";
        echo "</head>\n";
        echo "<body>\n";

    }

    /**
    *
    * I get $title from index.php
    *
    */
    public function setTitle( $title )
    {

        $this->title = $title;

    }


    __destruct{

        echo "</body>\n";
        echo "</html>\n";

    }

}
?>

You've probably noticed already that this code will produce a title of well, . Yah, there's an empty space there. :- )

To me this actually makes perfect sense (even if I wasn't expecting it) because the class constructor is being executed on the creation of the foo object meaning that it doesn't wait around for index.php to supply the argument for setTitle() which in turn returns the string that fills $this->title.

So if I truly understand whats going on here, what can I do to get around this issue? Should I be buffering the output using the built in buffer functions, and then modifying the stored output with the supplied title string? Any ideas?

I would really, really, really, like to keep this structure where the constructor and destructor contain this repetitive code. Its nice that these functions don't have to be called anywhere. I understand that some developers may consider this bad practice, but I am going to do it this way anyway because I want to because I think its cool. So i'm not really looking for advice in that aspect, unless you feel extremely motivated to inform me on my stupidity.

So, if you have any advice/ideas/knowlege to share with me that would be great.

Thanks, and feel free to take your time because I guess i'm going to be forced to stay inside hiding from the evil swine flu that has come to my city, so no rush!

From stackoverflow

  • You could do all the printing in the destructor. There all variables are known. However (like you said yourself) I think this is really really bad practice. I would suggest using kind of view / template files (you anyway have time enough :) ).

  • You could pass the title as an argument in the constructor?

    teh_noob : I could, but the whole idea is that the title is supplied by index.php (this is all part of a little framework i'm building).
    Schnalle : please, stop! don't build a framework that outputs html in the constructor. so wrong!
    teh_noob : I have to, some times you have to take one for the team to make a difference.
    Steven Richards : Gotta side with Schnalle and Peter here. The point of a constructor is to initialize an object, not output content to the browser.

  • schnalle: this is bad practice. this is madness.
    the_noob: madness ...?
    the_noob: (shouts) THIS ... IS ... LITTLEFRAMEWORKIMBUILDING!
    the_noob: (kicks separation of code and presentation down the well)

    i don't really get what you want to do, but just pass the title as a parameter to the constructor ...

    <?php

class Title {
    public function __construct($title) {

        echo '<html><head><title>' . htmlspecialchars($title) . '</title></head><body>';
    }

    public function __destruct() {
        echo '</body></html>';
    }
}

?>

    if you really want your objects to print something, i'd suggest the magic __tostring() method, so you can simply echo your object. but for html tags ... still not useful.

    i wish you luck with your framework, but you're either a genius (not likely) or a guy making the same mistakes (almost) every beginner did (before MVC arrived).

    edit: i can't help you. you want direct output when the object is created, but need to get data into the object before it is created. so you try to workaround something ugly in making it even uglier. it just doesn't work that way!
    you're trying to build a new, better kind of car by attaching wheels to a living donkey, then complain because it somehow didn't work out as you expected (donkey on wheels! wheooo!), and now you ask the community how to attach taillights in a way that makes the donkey/car go faster.

    teh_noob : LOL, yes I know i'm breaking all the rules here. But i'm done letting all these rules and standards boss me around! Its time to deviate from the dull pattern of web frameworks! So who's with me?!?!
    teh_noob : hmmmmm...interesting, however I don't like the idea of passing variables directly into a constructor like that (for no logical reason). However that gives me the idea that maybe I could pass parameters from other functions.
    teh_noob : In all seriousness, i'm no where close to a genius but i'm not dumb either. And what may come as a surprise to you, my framework is MVC based. I'm deviating from the rules a little bit though because I think it's better the way i'm doing it. You can and have already helped me. We must be seeing things from different angles because I see beauty in this. Don't hate me for asking for ideas.
    Schnalle : i don't hate you for that, i did a lot of crazy (and nonsensical) stuff myself. learned a lot from those attempts to do it the "my" way. and i agree, you can do stuff quite elegant by wrapping html around objects. but the constructors and destructors are NOT meant for this and it absolutley makes NO sense to try to force them into this role.
    teh_noob : Fair enough, but i'm still sticking with my way.

  • teh_noob. Listen to me. These words that I'm typing right here. Pretend I'm saying them, and hear the words that are coming out of my mouth. No. NO NO NO NO NO! And no, I don't give a rat's butt how "cool" you think it is.

    This is one of those unfortunate scenarios where it would be shorter to list the "right" things about this approach that the "wrong" things. That is to say, your problems are numerous. That being said, I'm going to go over only some of the things that are bad about this idea.

    First, let's just discuss OOP in general. What you are doing here is my least favorite thing to see ever: what I call "programming with classes". That is to say, structured programming in the guise of OOP because a class keyword was used. If you're gonna do this, don't bother. Just use functions. This is class abuse plain and simple.

    Classes are object blueprints.Objects lend themselves to encapsulation and instantiation. Unless you're really a fan of the Singleton pattern, why create a class that is clearly designed to be instantiated only once? And before you say "But Peter, the Singleton Pattern helps us!!!1one" try to understand that it's actually not all that great. Besides, what you're doing here isn't even a reason people turn to the Singleton pattern in the first place.

    Second is the topic of subclasses. Maybe at some point in the future you'll want some popup pages for your site. Or you'll want print-only versions that are more than just CSS-driven. Maybe you'll even want something that's not HTML at all like an RSS feed. What now? How much other work that goes on in this constructor are you going to have to duplicate for these new page types to work? But what if you've already started relying on subclasses to create individual pages? Now you're fucked. Sure, you can go back and hook up the decorator pattern, but why go through all that work when this problem can be avoided by non-stupid class design in the first place?

    Thirdly, is the idea of echoing HTML in the first place. I'm fine with echo for a word or two here, a tag or three there. But for large HTML chunks, it's just idiocy. Have the decency to escape to output mode and use HTML that's not locked down in a string. Not only is it easier to edit and read, you can actually work with it in a WYSIWYG should you so desire.

    Fourth, this badly, badly breaks the SRP.

    Fifth - this kind of ridiculous design leads to the very type of problems you're trying to solve here. Only you don't want to know that the solution is to remove the echo statements from your constructor. Is there a way around it? Sure. In fact, there's even more than one. Do I recommend any of them? No, not really.

    Lastly, let's discuss headers. Maybe you haven't learned about them yet. Maybe you have and don't care. But what's going to happen when, 6 months from now, you're working on a problem and you're working inside a method that's 10 calls deep in the stack and you realize a simple header() function will solve your problem. Maybe you need to adjust the cache-control or you need to manually set the response code - whatever. But guess what, you can't. Why? Because your silly constructor outputs to the browser the millisecond it's created.

    So, to recap: NO! Unless your actual, ultimate goal is to see some of your very on handiwork on The Daily WTF.

    What can I offer besides admonishment? Maybe part of a new direction? Well, debugging output is hard enough in well-built systems, so don't start by shooting yourself in the foot that does it implicitly. All output from your system should be explicit. If you want to make a "page" type of class, that's fine. Just don't do it like that.

    class foo
{
    protected $title;
    protected $headers;

    public function setTitle( $title )
    {
        $this->title = $title;
    }

    public function addHeader( $header )
    {
        $this->headers[] = $header;
    }

    public function sendHeaders()
    {
        foreach ( $this->headers as $header )
        {
            header( $header );
        }
    }

    public function printPageHeader()
    {
        $this->sendHeaders();
        ?>
            <html>
                <head>
                    <title><?php echo $this->title; ?></title>
                </head>
                <body>
        <?php
    }

    public function printPageFooter()
    {
        ?>
                </body>
            </html>
        <?php
    }

    public function printPage()
    {
        $this->printPageHeader();
        $this->printPageFooter();
    }
}

$p = new foo;
$p->setTitle( 'Just Testing' );
$p->addHeader( 'Cache-control: no-cache' );
$p->printPage();
    teh_noob : I wouldn't be being honest if I told you I agreed with everything you said there. But you do fall into the category of "extremely motivated", so I will award you the "answer" for this question. I think next time I ask a question such as this I will add more details as to the scope of my project so people will better understand what i'm getting at. Your rant made me tired i'm going to bed now. One last note my example above was only meant to help explain my question and was not actually in or a part of my framework.
    Peter Bailey : Well, I'd love to see more about what it is you're working on, and ACTUALLY trying to do.
    teh_noob : Don't drink mountain dew and then try to sleep. Maybe when i'm done ill send you a link to prove my stubbornness or everything will work fine without a problem because the code just works anyway. I could re ask my question as: "Help me find good ways to do something wrong because I know its "wrong" but I want to do it anyway because I like it that way and it works and it has an un-logical logic to it that only applies to this specific situation and its only a tiny part of the framework so it will not doom everything"
Posted by Mu Cat at 9:20 PM

0 comments:

Post a Comment

Subscribe to: Post Comments (Atom)