Page 1 of 2

OOP Best Practice? echo

Posted: Tue Jun 10, 2008 10:18 am
by psurrena
When using a class, is it proper to echo when calling?

Code: Select all

echo $peopleList->dbCheckList("membership","mem_id","mem_name");
Or if you are returning something that will be echoed, should you do it in the function?

Code: Select all

public function dbCheckList($table, $value, $title, $type="checkbox"){
    $this->table=$table;
    $this->value=$value;
    $this->title=$title;
    $this->type=$type;
    
    $result=mysql_query("SELECT {$this->value}, {$this->title} FROM {$this->table}{$this->orderBy}{$this->limit}") or die (mysql_error());
    if(mysql_num_rows($result) < 1){
        $this->errorMsg="There was no content found.";
    }else{
        while(list($value,$title)=mysql_fetch_array($result)){
            $html.="{$title} <input type=\"{$this->type}\" name=\"{$title}\" value=\"{$value}\" />\n ";
        }
    }
    return $html;
}
 

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 10:42 am
by hansford
I've seen it done both ways. I personally don't like letting functions echo anything unless it's specified a display function. Functions should either set or return. What if you only want the value that the function gets and not have it echoed out. if it just returns a value then you can decide whether to echo it out or not.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 10:59 am
by psurrena
This is definitely a display function, it lists checkbox items. So echo wouldn't be so bad?

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 11:13 am
by superdezign
Then it shouldn't be a display function. Typically,we try to separate view from logic, so since this functions accesses the database, it shouldn't also create HTML. You can do that, but that's not what would be considered 'Best Practice.'

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 11:46 am
by psurrena
So, it should be more like:

Code: Select all

public function dbCheckList($table, $value, $title){
    $this->table=$table;
    $this->value=$value;
    $this->title=$title;
    
    $result=mysql_query("SELECT {$this->value}, {$this->title} FROM {$this->table}{$this->orderBy}{$this->limit}") or die (mysql_error());
    if(mysql_num_rows($result) < 1){
        $this->errorMsg="There was no content found.";
    }else{
        while(list($this->itemId, $this->itemName)=mysql_fetch_array($result)){
            $display.=$this->checkItem();
        }
    }
    return $display;
}
 
public function checkItem($type="checkbox"){
    $this->type=$type;
    
    $html="$this->itemName: <input type=\"{$this->type}\" name=\"{$this->itemName}\" value=\"{$this->itemId}\" />\n ";
    return $html;
}
 
// orderBy / Limit functions below
 

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 12:13 pm
by Mordred
No, after the query, pull the data in member arrays of the object. In the "display" function, loop over the member arrays (and other members of course) and generate the output HTML. It will help you define the responsibilities better if you name the two methods something like dbInitFromDatabase and dbGenerateHtml.

As for your original question, yes, it is more flexible if the "display" function does not actually display it, but only place it in a string. You can always say echo $object->GenerateHtml() when you need to output it.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 12:26 pm
by superdezign
No, more like having that function return an array, and then having your view turn that array into HTML, not that same class.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 12:32 pm
by Mordred
Do not agree.
Separating view from logic is not the only concern of writing well-structured code. The data he pulls from the database looks like inherent data for the object (hard to tell from this interface, might not be the case actually) Suppose he wants to somehow alter the data before displaying. What do you suggest - pass it around as an array?

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 12:37 pm
by Christopher
I agree ... probably the most important separation you can make is between the Data Source and Presentation code (that's about 90% of MVC ;)). Having that method return an array of data would create that clear separation. Then another, separate object can format the array data into HTML.

Another thing you might want to think about is a Response object. You give generated output to it, plus any headers, redirects, etc. Then at the very end you output it and it can decide to echo the content or just send a redirect header. It formalized the Response and also gets rid of 'headers sent' errors.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 12:42 pm
by superdezign
Mordred wrote:As for your original question, yes, it is more flexible if the "display" function does not actually display it, but only place it in a string. You can always say echo $object->GenerateHtml() when you need to output it.
But to have a function that depends entirely on the running of another function seems like a waste to me. Unless this data is the primary existence of the class, then this would be better suited as a function that retrieved and returned the data rather than turning it into an integral part of the object.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 12:48 pm
by RobertGonzalez
Personally I do not echo anything in functions/methods. When it comes to OO design I tend to leave the echoing up to the views.

I know it was mentioned earlier that you could build an html string inside the function and return that. But what happens if that same data needs to be outputted into a spreadsheet? What about a CSV? Or an email? Better yet, a CLI app? Are you going to want HTML in that?

That is why the ultimate layer that displays should be responsible for pushing content to the end user. The data should be fetched/set/handled somewhere with the results of that process returned, or in the case where there is not return to return, a boolean value can be.

Just my two cents.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 1:31 pm
by psurrena
Not sure if I get what everyone is talking about. But what I'm gathering from it all is, we have the class below return an array.

Code: Select all

class NewClass{
    public function dbCheckList($table, $value, $title){
        $this->table=$table;
        $this->value=$value;
        $this->title=$title;
        
        $result=mysql_query("SELECT {$this->value}, {$this->title} FROM {$this->table}{$this->orderBy}{$this->limit}") or die (mysql_error());
        if(mysql_num_rows($result) < 1){
            $this->errorMsg="There was no content found.";
        }else{
            $this->row=mysql_fetch_array($result);
        }
        return $row;
    }
Then on a page that would contain html code(view?? page) we would have

Code: Select all

<p>Please choose from the list below:</p>
<?php
$test= new NewClass();
$test->dbCheckList("membership","mem_id","mem_name");
 
foreach($test->row as $value){
    //check box code
     echo $value;
};
?>

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 1:36 pm
by RobertGonzalez
That is one way to think of it. Or you could have the database stuff in a data handler/model object that is built by a controller (page/action/whatever controller) that feeds the $model->method() result to the view as a variable wherein the view does what it wants with that data.

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 1:45 pm
by psurrena
how would a data handler differ from what I have?

Re: OOP Best Practice? echo

Posted: Tue Jun 10, 2008 1:52 pm
by RobertGonzalez
It might not. Would all of the database interaction for a given controller be in the class you are using?