Is it reasonable to have a fair amount of public properties in a class?

Question

Or in more specific words, is it "ok" to not be relying on setters and getters?

I'm dealing with a class that checks the availability of rooms and sets public properties of which there are more than a dozen. Things such as:

• unitNumber
• roomTypes ( array )
• codeCorporate
• codeGroup
• numberKids
• numberAdults
• numberRooms
• currency
• minRate
• maxRate
• soapServer
• units ( array )
• hotelId

And after an object is instantiated those properties are set with $this-> inside various methods. However the code that deals with the object often sets public properties directly instead of using getter/setter methods:

$object->something = 3;

foreach ($object->things as $thing ) { }

If I have the time to refactor this class..

• Should I stick all of these properties in a data array that's a private property, and define __set and __get methods?
• Should I make a single getter method for each of the properties?

Answer 1

I personally have yet to find a truly good reason for a public property, though im open for suggestion :-)

Although i much prefer specified getters/setters for each property (whether that's a proxy to a generalized get($name) or not). I assume you have other code already that uses direct assignment so in that case i would say to proceed with using the magic __get/__set methods.

Answer 2

In my opinion, it is rarely a good idea to have any public members. It increases coupling between classes, and makes refactoring very complicated (should you need it.)

Setters/Getters are the way to go, and the very small performance penalty that you pay for it is usually either optimized away, or trumped by elegance.

To answer your question about array vs. single-getter-per-var, it's a matter of taste. I tend to only keep variables of a similar type within an array, and separate the rest.

Answer 3

I think most people will recommend using setters & getters. Right now you're limited to simply setting & fetching the property, but what if you want to log when that property is accessed? Or perhaps you want to run the value by a validation function first (email, phonenumber, zip code, etc). Maybe you'll need to call another function, or set another property. I think you see where I'm heading with this. By using setters & getters, you add a valuable layer of encapsulation to your classes, and 99% of the time this is worth the extra typing you'll need to do ;) Imagine trying to do the examples above without setters & getters. It'd be a big headache to say the least.

Edit: I forgot to mention Doctrine. It's an object relation mapper (ORM) that can automatically setup setters & getters for you (amongst other things). You can check it out at http://www.doctrine-project.org/

Answer 4

I would take a step back and ask some more general questions:

• Why am I having to expose this much information; what is using it and why?
• Is this class really just a data structure without behavior, in which case should be a private class to some other class?
• Does this class serve a single purpose, or is it on the path to becoming monolithic?

You may discover that you are able to create views of an instance of a class to export to a database, display in a form, etc. Check out the "Builder" and "Acyclic Visitor" patterns to start with.

Regarding accessors, I do not see a need to use them for what you are describing: retrieving class properties and internal state information, aka a struct. However, for attributes of a class I could see the benefit in certain cases, but more for retrieving attributes, not for mutations of your object's state.

Answer 5

If I may add my grain of salt several months later :

It is very un-OO to have public properties. Everything should be encapsulated, simply because (among other reasons) using direct attribute manipulation doesn't give you ways to easily refactor or perform (more) control checks when some external source modifies the field. For example, let's say you have a class with many fields that is used throughout a project several times, and that project contains several thousands of files; it's a project that has been running and expanded for a few years now. Let's say that the company is changing it's business model, or that a problem is found with some of the field's data type and now is required to have some validation; will you duplicate that validation in all those thousands of source code that is directly accessing the public member? In PHP, the solution may be simple, but not in most OO programming language (i.g. Java). The fact is that OO is based on encapsulation. In short, encapsulation doesn't only produce clean code, but also highly maintainable (not to say cost-effective and cohesive) code.

Your suggestion of having a private member (array) being manipulated by __get / __set is good. This way, if you need some extra validation along the road, simply create your setter and/or your getter and it will be the end of it. Some may argue with that being counter productive as the code completion cannot kick-in on __get / __set. IMHO, relying on code completion is simply lazy coding. But then again, having every member have it's own getter and/or setter allows you to write a more comprehensive API documentation. Personally, I usually use that technique for internal or very general purpose classes. If all your fields do not require any validation, or there are as you said several dozen of them, then using magic methods would be acceptable, in my opinion.

The bottom line is to avoid direct member access on class instances, period. How you decide to achieve this is strictly up to you. Just make sure that the API is well documented the more abstract you make it.

On a final note, in PHP, if you already have classes that are being used that are not encapsulating their fields, for example something like

class SomeObject {
   public $foo;
   public $bar;
   public $baz;
   //...
}

you can simply fix this class without having to refactor anything with something like :

class SomeObject {
   private $_foo;   // having underscore as prefix helps to know what's private/protected
   private $_bar;   // inside the code.
   private $_baz;

   public function __get($name) {
      $methodName = 'get'.ucfirst($name);
      if (method_exists($this, $methodName)) {
         return $this->{$methodName}();
      } else {
         throw new Exception("Metohd '{$methodName}' does not exist");
      }
   }

   public function __set($name, $value) {
      $methodName = 'set'.ucfirst($name);
      if (method_exists($this, $methodName)) {
         $this->{$methodName}($value);
      } else {
         throw new Exception("Metohd '{$methodName}' does not exist");
      }
   }

   public function getFoo() { return $this->_foo; }
   public function setFoo($value) { $this->_foo = $value; }

   public function getBar() { return $this->_bar; }
   public function setBar($value) { $this->_bar = $value; }

   public function getBaz() { return $this->_baz; }
   public function setBaz($value) { $this->_baz = $value; }

}

And then

$obj = new SomeObject();
$obj->foo = 'Hello world';    // legacy code support
$obj->setFoo('Hello world');  // same thing, but preferred

And you satisfy both the OO paradigm and having direct access to attributes of an instance. You could also have __call() check for prefix 'get' or 'set' and call __get() and __set() accordingly, but I would not go that far, though this would truly enable general purpose classes to access it's private members via ->member and ->getMember()/->setMember()