Why get/set methods?

[Dangermouse]

What is the point of this for example

class Person
{
var $name;

function setName($name)
{
$this->name = $name
}
function getName()
{
return $this->name;
}
} 

Ive been told many times its 'good practise' to have get/set methods for your variables, but thats a whole lot of functions if you have a lot of variables

[McGruff]

The whole point of a class is to encapsulate something behind an interface. Although in php you can access class properties directly, getter and setter methods create a well-defined interface. You can see at a glance how the class is used.

Often a good way to write a class, or a group of classes which work together, is to start by writing down the interface methods then fill in the details later.

[lastcraft]

Hi...

Ive been told many times its 'good practise' to have get/set methods for your variables, but thats a whole lot of functions if you have a lot of variables

There two things that factor in to this. The first is that if you have to change from member access to an accessor later you will have to change the client code in an awful lot of places. The interface is king. A few extra lines of code pales into insignificance by comparison.

The other thing you have to ask is what the hell is name doing in this class? It should be moved to the classes that actually use it, or the operations should be moved into this class. In fact a large number of accessors is a sign that the code has not been factored very well and some shuffling is in order. The data and methods need to be migrated together.

There is an OO rule of thumb that goes "tell, don't ask". In other words, get your objects to do things, don't keep asking them for things so that you can do them on their behalf. Every time you ask an object about it's state you ever so slightly break encapsulation by exposing it's underlying design.

yours, Marcus

[Widow Maker]

I use Setters and Getters all the time, and I cannot for the life of me disagree with either Marcus nor McGruff

[Dangermouse]

Thanks for the insight guys, i havent looked into design methods or patterns at all.

[Widow Maker]

Hello again, here is what I am using just now. The script was posted on another thread the other day there, but here it is again.

Hope you can (from this script) understand the important of using Setters and Getters, ie If the database field names had to change, I'd only need to alter the Setters and Getters, and not the script (client) that actually uses the ''Archives''

# modules/news/news.class.php

<?php

    class NewsGateway {
        function NewsGateway() {
        }
        
        function FindAllWithAuthor( &$db ) {
            $findAllWithAuthor = "SELECT 
                fld_id,
                UNIX_TIMESTAMP( fld_date ) AS fld_date,
                fld_body,
                fld_title,
                fld_status FROM tbl_news ORDER BY fld_date DESC";
            
            return $db -> Execute( $findAllWithAuthor );
        }
        
        function FindById( &$db, $id ) {
            $findById = "SELECT 
                fld_id,
                UNIX_TIMESTAMP( fld_date ) AS fld_date,
                fld_body,
                fld_title,
                fld_status FROM tbl_news WHERE fld_id = '".$id."'";
                
            return $db -> Execute( $findById );
        }
    }
    
    class NewsManager {
        function NewsManager() {
        }
        
        function Load( &$resultset, $archiver ) { 
            $indexer = array();
            $iterator = &new QueryIterator( $resultset );
            
            while( $iterator -> IsValid() ) {
                $row = $iterator -> GetCurrent();
                $iterator -> GetNext();
                #
                $archive = &new $archiver; 
                $archive -> Invoke( $row );
                
                $indexer[] = &$archive;
            }
            
            return $indexer;
        }
    
        function Update() { } // update database records
        
        function Insert( &$db, $title, $body, $status ) { 
            $sql = "INSERT INTO tbl_news SET
                fld_date = now(),
                fld_body = '".$body."',
                fld_title = '".$title."',
                fld_status = '".$status."'";
                
            $db -> Execute( $sql );
        } 
        
        function Delete() { } // delete database records
    }
    
    class NewsArchiver {
        var $indexer;
        
        function NewsArchiver() {
            $this -> indexer = array();
        }
        
        function SetId( $id ) {
            $this -> indexer['id'] = $id;
        }
        
        function SetDate( $date ) {
            $this -> indexer['date'] = $date;
        }
        
        function SetBody( $body ) {
            $this -> indexer['body'] = $body;
        }
        
        function SetTitle( $title ) {
            $this -> indexer['title'] = $title;
        }
        
        function SetStatus( $status ) {
            $this -> indexer['status'] = $status;
        }
    }
    
    class FindAllWithAuthorArchive extends NewsArchiver {
        function FindAllWithAuthorArchive() {
            NewsArchiver::NewsArchiver();
        }
        
        function Invoke( $row ) {
            $this -> SetId( $row['fld_id'] );
            $this -> SetDate( $row['fld_date'] );
            $this -> SetBody( $row['fld_body'] );
            $this -> SetTitle( $row['fld_title'] );
            $this -> SetStatus( $row['fld_status'] );
        }
        
        function GetId() {
            return $this -> indexer['id'];
        }
        
        function GetDate() {
            return $this -> indexer['date'];
        }
        
        function GetBody() {
            return $this -> indexer['body'];
        }
        
        function GetTitle() {
            return $this -> indexer['title'];
        }
        
        function GetStatus() {
            return $this -> indexer['status'];
        }
    }
    
    class FindByIdArchive extends NewsArchiver {
        function FindByIdArchive() {
            NewsArchiver::NewsArchiver();
        }
        
        function Invoke( $row ) {
            $this -> SetId( $row['fld_id'] );
            $this -> SetDate( $row['fld_date'] );
            $this -> SetBody( $row['fld_body'] );
            $this -> SetTitle( $row['fld_title'] );
            $this -> SetStatus( $row['fld_status'] );
        }
        
        function GetId() {
            return $this -> indexer['id'];
        }
        
        function GetDate() {
            return $this -> indexer['date'];
        }
        
        function GetBody() {
            return $this -> indexer['body'];
        }
        
        function GetTitle() {
            return $this -> indexer['title'];
        }
        
        function GetStatus() {
            return $this -> indexer['status'];
        }
    }
    
?>

The above is used by this parts

class ShowNewsController {
        var $view;
        
        function ShowNewsController() {
        }
        
        function GetPage() {
            return $this -> view;
        }
        
        function Execute() {
            $id = (int) $_GET['id'];
            
            require_once( 'modules/news/news.class.php' );
            
            $db = singleton::getInstance();
            $model = NewsManager::Load( NewsGateway::FindById( &$db, $id ), 'FindByIdArchive' );
            
            $this -> view = &new ShowNewsView( &$model );
        }
    }
    
    class ShowNewsView extends BasePageView {
        var $model;
        var $title = 'Administer your sites news';
        var $heading = 'News :: View Item';
        
        function ShowNewsView( &$model ) {
            BasePageView::BasePageView( 'templates/show-news.template.htm', 'templates/basic.template.top.htm' );
            
            $this -> model = &$model;
        }
        
        function Finalise() {
            $this -> Paste( '<page_body />', $this -> Fetch() );
            $this -> Paste( '<page_heading />', $this -> heading );
            
            $iterator = &new ArrayIterator( $this -> model );
            $row = $iterator -> GetCurrent();
            
            $this -> Paste( '<page_news_id />' , $row -> GetId() );
            $this -> Paste( '<page_news_body />', $row -> GetBody() );
            $this -> Paste( '<page_news_date />', date( DATE_LONGFORMAT, $row -> GetDate() ) );
            $this -> Paste( '<page_news_title />', $row -> GetTitle() );
            $this -> Paste( '<page_news_status />', $row -> GetStatus() == (int) 1 ? 'Yes':'Pending' );
        }
    }
    
    class AddNewsController {
        var $view;
        
        function AddNewsController() {
        }
        
        function GetPage() {
            return $this -> view;
        }
        
        function Execute() {
            if( isset( $_POST['BasicSubmit'] ) ) {
                # user has requested to add a news item
                $body = (string) addslashes( $_POST['Body'] );
                $title = (string) $_POST['Title'];
                $status = (int) isset( $_POST['Status'] )? 1:0;
                
                $db = singleton::getInstance();
                
                require_once( 'modules/news/news.class.php' );
                # implement Model layer
                NewsManager::Insert( &$db, $title, $body, $status );
                
                $this -> view = new AddNewsViewSuccess;
            }
            else {
                $this -> view = &new AddNewsView( &$model );
            }
        }
    }
    
    class AddNewsView extends BasePageView {
        var $model;
        var $title = 'Administer your sites news';
        var $heading = 'News :: Add Item';
        
        function AddNewsView( &$model ) {
            BasePageView::BasePageView( 'templates/add-news.template.htm', 'templates/basic.template.top.htm' );
            
            $this -> model = &$model;
        }
        
        function Finalise() {
            $this -> Paste( '<page_body />', $this -> Fetch() );
            $this -> Paste( '<page_heading />', $this -> heading );
        }
    }
    
    class AddNewsViewSuccess extends BasePageView {
        var $title = 'Administer your sites news';
        var $heading = 'News :: Add An Item';
        
        function AddNewsViewSuccess() {
            BasePageView::BasePageView( 'templates/add-news-success.template.htm', 'templates/basic.template.centre.htm' );
        }
        
        function Finalise() {
            $this -> Paste( '<page_body />', $this -> Fetch() );
            $this -> Paste( '<page_heading />', $this -> heading );
        }
    }
    
    class NewsController {
        var $view;
        
        function NewsController() {
        }
        
        function GetPage() {
            return $this -> view;
        }
        
        function Execute() {
            require_once( 'modules/news/news.class.php' );
            
            $db = singleton::getInstance();
            $model = NewsManager::Load( NewsGateway::FindAllWithAuthor( &$db ), 'FindAllWithAuthorArchive' );
            
            $this -> view = &new NewsView( &$model );
        }
    }
    
    class NewsView extends BasePageView {
        var $model;
        var $title = 'Administer your sites news';
        var $heading = 'News :: Administration';
        
        function NewsView( &$model ) {
            # defaults, $page = templates/admin.body.htm,
            # defaults, $template = templates/basic.template.centre.htm
            BasePageView::BasePageView( 'templates/news.template.htm', 'templates/basic.template.top.htm' );
            
            $this -> model = &$model;
        }
        
        function Finalise() {
            $this -> Paste( '<page_body />', $this -> Fetch() );
            $this -> Paste( '<page_heading />', $this -> heading );
            
            $html = '';
            $iterator = &new ArrayIterator( $this -> model );
            
            while( $iterator -> IsValid() ) {
                $row = $iterator -> GetCurrent();
                $iterator -> GetNext();
                #
                $content = &new ViewHelper( 'templates/modules/news.template.htm' );
                
                $content -> Paste( '<page_news_id />', $row -> GetId() );
                $content -> Paste( '<page_news_date />', date( DATE_LONGFORMAT, $row -> GetDate() ) );
                $content -> Paste( '<page_news_published />', $row -> GetStatus() == (int) 1 ? 'Yes':'Pending' );
                $content -> Paste( '<page_news_title />', $row -> GetTitle() );
                
                $html .= $content -> Finalise();
            }
            
            $this -> Paste( '<page_news_list />', $html );
        }
    }
    
    Session::Start();
    
    $page = &new Page;
    $page -> Dispatch(); 

Enjoy

[mmj]

Simply put, getters and setters give you the opportunity to modify the way the class data members work without breaking compatibility. You simply modify the getter and setter.

[Dangermouse]

Widow, thanks for confusing the hell out of me

mmj, yea i get it now, thanks!

[lazy_yogi]

In fact a large number of accessors is a sign that the code has not been factored very well

The exception is your DB access classes where you have to set or get all attributes depending on you query.

$person = new Person();
$person->name = "blah";
$person->age = "25";
PersonTable::Insert($person); 

And alot of the dynamic content on the web is centered around db access.

On a related note, I was surprised to see php5 has 'properties', so you don't have to use

$this->get_var() 
$this->set_var() 

you can just use

$this->Var 

Of course you have to create the method __get(), but looks a nicer interface to the class attributes.

[Azmo]

Why getter and setter methods are evil by Allen Holub.

While I don't always agree with Allen, he does have a point in this case. Some people use getters and setters out of habit, not out of need, and this seems bad OO practice to me. I don't think encapsulation is achieved by creating (indirect) access to object members.

I try to avoid using getters and setters for most class members, except for those cases where it really makes sense to use them.

[Widow Maker]

Though from what I can remember, there wasn't really a proven solution viable enough not to use Getters and Setters either

A Holub btw, has another article, over at Java World about the evils of class extension which is worth having a read of as well

And alot of the dynamic content on the web is centered around db access.

And the only way around it is to use Getters and Setters, though I might add, not just DB access alone, as you may find at one point that you'd need to use Getters and Setters in relation to flat files as well, such as XML

In the classes I've posted, the Base Archiver has all the Setters required, one per database field name, although the sub-class Archiver really only needs to implement the Getters that you need to use for that given function, ie News - One point you may only need the ID and Date, nothing more, so you do not need to have Getters for the remaining (defunct) field names

Took me a minute to work out the best way of achieving that actually

It works though, and I'm happy with it

[sobri]

I use setters and getters frequently, but the one place I find I can't is when the property is an object. ie:

$reg =& Registry::load();
if ( $reg->getTask() == 'CreateAccount' ) { ... } 

works fine, but in php4, this does not:

$reg =& Registry::load();
if ( $reg->getUser()->username == 'admin' ) { ... } 

My understanding is that in php5 this will work?

[momos]

Why getter and setter methods are evil by Allen Holub.

Nice article...

[dgm]

Yeah, the inability to call methods like this:

$foo->Bar()->Baz() 

is frustrating.

[HarryF]

Aside from any issues of good / bad design, accessing object properties via a method has a very significant performance overhead, compared to accessing them directly. Try the following (using XDebug) for a rough idea;

<?php
class Test {
    var $foo;
    function setFoo($foo) {
        $this->foo = $foo;
    }
    function getFoo() {
        return $this->foo;
    }    
}
$T = & new Test();
xdebug_start_profiling();
for ($i=0;$i<100;$i++) {
    $T->foo = 'bar';
    $foo = $T->foo;
    // $T->getFoo('bar');
    // $foo = $T->getFoo();
}
xdebug_dump_function_profile();
?>

For me, accessing the properties directly, I get;

Opcode Compiling        0.0076329708
Function Execution      0.0000000000
Ambient Code Execution  0.0001561642
Total Execution         0.0001561642
Total Processing        0.0077891350

Using the setters / getters instead I get;

Opcode Compiling        0.0078232288
Function Execution      0.0003659725
Ambient Code Execution  0.0035359859
Total Execution         0.0039019585
Total Processing        0.0117251873

For something like populating a 5 column table with 50 rows, using "dumb" setters and getters has a significant cost.

Of course I don't think you can make a blanket "setters and getters are evil" statement without qualification but perhaps a rule of thumb is if they don't modify the data they are setting / getting, perhaps they need review (bearing in mind the API issues Markus mentioned).

 $foo->Bar()->Baz() ; 

...it's available with PHP5.

[McGruff]

I feel a need to speak up some more for getters and setters.

To me, the three R's of programming are Readability, Readability, and Readability. The main thing I have in my mind when writing a class is how to make it intuitively obvious what's going on. Maybe someone else will have to work with it one day; or, when I get a call from a client with a problem or a request for an addition to the site in six months time (ie when I've forgotten all about how the class works) I'll have to pick it up again quickly.

Getters and setters aren't evil (I think the Holub article wasn't against getters/setters exactly but rather badly designed classes which don't encapsulate aspects of the system as well as they might). They are very important in allowing you to see at-a-glance what a class does and how it should be used. Often you won't need to look any further than that. If you are going to encapsulate something behind an interface you have to have a well-defined interface.

I haven't used xdebug but, assuming the two relevant numbers above are the total execution times, the difference is a few thousandths of a second. That's interesting, and more than I would have guessed, but off the radar imho. People new to OOP often worry about "inefficiency" but I don't think this should be a concern.

[Widow Maker]

but I don't think this should be a concern.

Not with todays technology, and designing for future refactoring and readability does play an important role in development

[firepages]

I still use dynamic getters and setters , and until PHP5 gives you more control over what you can and can not do with your class variables its not enforceble anyway.

and when it is ... yes those thousands of a second are worth it , especially in totally OO frameworks where such things are going on all over the place ... unless the result is itself `compiled to be interpreted` code etc.

JAVA has been telling us for years how fast JAVA will be ok as soon as we get the CPU ...

PHP!=JAVA

[ghurtado]

It seems to me that most of the problems that are being tagged on accessor methods is the fact that they promote a high degree of coupling between classes. This is most evident when you use a high number of "dumb" accessors that dont really do much, within a class that has a lot of member variables. You end up with unnecesary exposure of the types and names of the class members, and once you get to this point, accessors are essentially useless, no different than accessing the private members directly.

On the other hand, there must be some reasonable compromise between using accessors and avoiding high coupling. Myself, for quite some time now, have thought that the best practical compromise is to use generic accessors as opposed to specific ones, much like the ones described in this thread http://www.sitepoint.com/forums/show...ghlight=setter

In doing so, you still have room left to manouver should you have to change the names of your private members, and could simulate the same benefits of "old-school" accessors by introducing a mapping strategy. This strategy can be as simple as an array that maps "outside" names to internal member variable names (case which I use to translate between the "code names" of variables and the "DB names") or a much more complex strategy involving filters, actions and whatnot. The biggest advantage besides the decoupling is that you end up trimming your code tremendously (specially with value objects), since you only have one getter and setter per class, not one per member variable.

I have mentioned this style of accesors in a few threads, and after using them in the real world, I am most happy with the results so far.

[manoloweb]

Will this work? Is it appropiate / good practice?

class Foo() {
    var $one;
    var $two;
    var $three;
 
    function Foo() {
    }
 
    function getValue($var) {
        return $this->$var;
    }
 
    function setValue($var,$val) {
        if (isset($this->$var)) {
            $this->$var = $val;
            return true;
        } else {
            return false;
        }
    }
} 

[iseff]

For something like populating a 5 column table with 50 rows, using "dumb" setters and getters has a significant cost.

Off topic: If you have a 50 column table, you might want to rethink your relations. I've never had a schema with more than 25 columns and I've done some large work.

Just a thought.

Ian

[Widow Maker]

if (isset($this->$var)) {
            $this->$var = $val;
            return true;
        } else {
            return false;
        } 

Would work, though as I see it, I'd try to leave the checking outside the Setter myself, and have the client script accessing the Setter, verify the value it's self, no ?

[dagfinn]

Why getter and setter methods are evil by Allen Holub.

I'm not saying he's wrong that many getters and setters are unnecessary and indicate poor design, but:

...an experienced OO designer could probably eliminate 99 percent of the accessors currently in your code without much difficulty.

He's having a great day, insulting his readers and damaging his own credibility, all in one sentence. <comments removed by pippo>. If he had provided a wealth of examples, he might convince me, but he doesn't.

Also, most of his argument against getters and setters centers seems to center around typing issues that are not very relevant to PHP.

[pippo]

Dagfinn try to post your comments with a bit of more ``elegance'', thanks.

[HarryF]

Off topic: If you have a 50 column table, you might want to rethink your relations. I've never had a schema with more than 25 columns and I've done some large work.

That's 5 columns and 50 rows if you re-read my post (not vice versa)

If the getters are not doing anything of value, that's 250 function calls which has alot more overhead that accessing 250 object properties directly.

Marcus hits the nail on the head here though. While development is in progress, using getters / setters gives you the option of being able to rename / remove a property, reflect the change in the setter / getter and leave all other code untouched. Once the design as settled, removing / rewriting dumb setters / getters that are redirecting data is probably a worthwhile exercise.