The SitePoint Forums have moved.

You can now find them here.
This forum is now closed to new posts, but you can browse existing content.
You can find out more information about the move and how to open a new account (if necessary) here.
If you get stuck you can get support by emailing forums@sitepoint.com

If this is your first visit, be sure to
check out the FAQ by clicking the
link above. You may have to register
before you can post: click the register link above to proceed. To start viewing messages,
select the forum that you want to visit from the selection below.

REMIYA, what you are saying is different from what Athox is saying. You are talking about flexibility, about attributes being private or protected. On the other hand, Athox is suggesting that there should be NO flexibility and that all the attributes should be private.

Athox is wrong, the following example will not work because of the private attributes:

If changed to protected, it works. That's what in php is called overriding and looks like he's finding it difficult to understand how polymorphism works in php, and why they recommend using protected. Yes, you have 2 ways of setting default values, and the other one is using setters and getters. That's the beauty of php, it's flexible. And because of that, you need to follow some standards. It's not about how you think it should be done, it's about how is done using php. So, stick to the standards, make all the attributes protected by default and write your code based on the different styles and options so it's flexible enough for others to use.

It's not a bug, although I can't recall why it happens. (I did get a sort of explanation from Ilia once.) Calling PDOStatement::setFetchMode() with PDO::FETCH_PROPS_LATE causes properties to be set only when they're accessed. I did some experiments with this about a year ago and found that in this case they're set after __construct(), but Ilia's explanation to me suggested that they might not even be set until they're accessed like $obj->prop. (In retrospect, this seems a bit strange because that may happen long after PDOStatement::fetch() or fetchAll() spits out the object.)

Good thing that constant is documented in the manual... oh what, it's not. In fact, the only thing of use googling turns up is a couple of posts you made on this site a while back.

In any case, if it's not a bug, it's certainly NOT intuitive behavior, and it just proves my point; that in PHP, it's possible for an object to have state before the constructor is called.

I think that there two good reasons to keep most attributes private. Private attributes assist versioning and refactoring. Refactoring include renaming the private attributes to reflect better understanding of the code. Refactoring also allows the type and/or data structure of the attribute to change for the purpose of optimisation or further simplification of the code. If the said attributes were protected or public, more code needs to be examined to perform the same task of refactoring. Even worse, not probably infeasible, if those code is directly out of reach for refactoring, e.g. used by 3rd parties.

Versioning closely follows refactoring as the design of the class evolves. This may include providing accessors and mutators to the private attributes as need arise. The user of the class should only need to understand the public API. In particular, a public method/attribute should be an unbreakable contract for the class if there is need to keep backwards compatibility (this is one aspect of versioning). This requires that each public method/attribute be carefully named and designed. Promoting a private attribute and/or method to protected or public must require careful consideration. The less protected/public attributes/methods are available, the easier it may be to permit versioning of the code.

These features, versioning and refactoring, become very important as the code base become large and age.

It is of course important that there should be a clearly defined interface and that this should be clearly labelled but it doesn't need to be enforced. The risk that people might wrongly call the wrong (notionally private) methods is hugely over-exaggerated IMO. It's so incredibly unlikely that you might as well say that it doesn't exist.

I rather like public and protected (don't see the need for private), at least for class variables. I can see that the need for those declarations in methods is less. To an extent, though, I'm with McGruff when he says...

it doesn't need to be enforced

Golly, if you're going to use someone else's code you'd better become reasonably acquainted with what it does or of course you'll get unexpected results. Though he hasn't said so, I suspect McGruff is frustrated by those who suggest that the author of a bit of code ought to be responsible for the (ab-)use its users make of it.

It is of course important that there should be a clearly defined interface and that this should be clearly labelled but it doesn't need to be enforced. The risk that people might wrongly call the wrong (notionally private) methods is hugely over-exaggerated IMO. It's so incredibly unlikely that you might as well say that it doesn't exist.

I agree with you, if we're talking about methods. However, we're also talking about attributes, and there's a real danger to having those be public, as it violates encapsulation. The need to maintain encapsulation comes above all else, as it will bite you in the *** somewhere down the line.

REMIYA, what you are saying is different from what Athox is saying. You are talking about flexibility, about attributes being private or protected. On the other hand, Athox is suggesting that there should be NO flexibility and that all the attributes should be private.

First of all, I didn't say that all attributes should be private. I said they should be as private as possible, which means that if you can avoid them being protected, you should.

Athox is wrong, the following example will not work because of the private attributes:

The only time you should give the attributes values directly at declaration is if they have an empty array() or null, and null is mostly not even needed to be set except in rare circumstances, or to give a hint at how the attribute works (say it holds a singleton instance, then a null value would indicate that it has not been initialized yet).

No one is making you do this, it's up to you. But it's for your own improvement if you follow that guideline. Because it provides a more stable coding model down the road.

Making all attributes public is just silly. For a quick and dirty it will work, but anything decent should properly use getters and setters. And if you find yourself writing too many of them, then you probably need to rethink your design.

I agree with you, if we're talking about methods.

That doesn't make sense. The same applies to methods, for the same reason. If you have a utility method in your class that is only internally used, but somewhere down the road you end up using it externally. Now later when you make a change to that internal method you will break the existing usage.

Don't think you wouldn't use that method, because when you are in a time pinch you will use whatever works quickest.

Athox, you are not getting the point. In php you can do it both ways. Saying that you shouldn't set values to attributes directly doesn't make sense. What you are saying is: "I'm right, everyone is wrong". And by saying that, you are suggesting that all the php frameworks should be rewritten.

Maybe that's how you do it, and it's fine. But please, don't bring strictness to a language that's completely the opposite.

In my personal experience, protected methods often end up somewhere in helper classes or functions because they provide methods that don't really fit the concern the class is trying to cover. I also hardly have any protected variables because I simply avoid using inheritance.

The only 'real' problem I have with the protected keyword is that you're limiting the data to subclasses and are thus forcing someone that needs the data to inherit, so the data I end up marking as protected ends up public anyway because I don't want to force anyone to inherit. Private doesn't have this problem, because most of the time the data marked as such is irrelevant elsewhere.

There is nothing wrong with the protected keyword, however, because it certainly has it's uses. However, what I described above is the way my codebases usually seem to evolve.

Jasper, that's a very good reason of why you should use private. If you want to avoid inheritance, you should definitely use it.

I use private in rare circumstances, I always use setter to assign values, and if I need to, I also assign default values to attributes, depending on the design of the class.

I think it's not about using one or the other, is about using the one that fits best to a given design. I'm against forcing the user to adapt to my class, I prefer a flexible design where the class adapts to the user. Because the truth is that we don't know how he's going to use it.

IMO, a good design should be flexible enough to adapt to different needs. Someone that designs a class should not base the design on personal preferences. E.g.: never set values in attributes, don't use inheritance, etc.

Private reduces the options, while protected opens up the possibilities.

Just a thought; making a varible public can screw things up, but it's also kind of annoying to write getters for every variable. How about having another definition, a readonly sort of thing? That means you can read a class variable, without having to create a getter, but it reduces the risk of another object screwing up the object. This could also be done by setting the variable to private/protected, and using __get() to return the variable, so that:
echo $obj->var; works, but $obj->var = 'asd'; won't work.

I suspect McGruff is frustrated by those who suggest that the author of a bit of code ought to be responsible for the (ab-)use its users make of it.

Up to a point. We do have to think how to make code simple to use and extend but I think it's OK to assume a basic level of competence. I haven't ever seen anyone mistakenly try to call a notionally private method in php4 before we had all this access stuff. We do need to clearly label the public interface but I don't think we need to enforce it.

I'm very much a test-infected programmer. Write a test; write the simplest code to pass the test; now move on. Test-first means that you don't write anything except in response to a real requirement (expressed by the test). The "simplest thing" mantra is very important. Code is harder to refactor once you start accumulating unnecessary complexity. It's harder just to read it before you even get started analysing how well you've cut the OOP cake.

You have to be ruthless: if there's anything which can be cut out without affecting functionality (ie all the tests still pass) it must be cut out. If you don't do that you'll gradually gather up all kinds of baggage. Complexity tends to breed yet more complexity and our task is to cut through that, writing the simplest, clearest code we can think of.

Unless I have a failing test (a desired behaviour which hasn't yet been implemented) which can only be made to pass by adding an access modifier, I won't add them - and I cannot think of any real requirement for access restrictions which can be expressed in the language of the domain. The last bit is important: you could of course write a test: "testNobodyCanCallMyPrivateMethod" but this sounds like it's testing implementation not behaviour.

I joined a new project recently which has a full suite of tests. Guess what happens when you remove all the access stuff? Absolutely nothing All the tests still pass.

Jasper:
So do you set your class properties to public, och do you have public getters for them and set the properties to private/protected?

I use them whenever I need them; however I feel that they are bad design because they are not defining behavior. But sometimes they're needed because you have some sort of dirty flag to set or other actions that need to be performed whenever a variable changes. Another reason is that the data exposed in this way, can be defined in an interface and the classes that use the interface are now decoupled from the implementation.

Originally Posted by phpimpact

Jasper, that's a very good reason of why you should use private. If you want to avoid inheritance, you should definitely use it.
[..]
E.g.: never set values in attributes, don't use inheritance, etc.

Not using inheritance is not so much a preference as it is a necessity for good design in my opinion. In the optimal case, all polymorphism happens though an interface because it reduces coupling. There are cases where that option seems wrong because it introduces code duplication, however you can always refactor the duplicate code into a separate class.

One example of this, is that I had modeled my controllers as a interception filter, each class had a next() method that stored a reference to the next controller in the list and that every execute() method had to determine if there was another controller on the chain and call it if it existed.

This is duplicate code and you really don't want that (that and because has O(n) performance, which might some day become a problem). Solving this involved refactoring all (!) the controllers and separating the traversal algorithm, which one might say, is a different concern anyway. The next big refactoring will be to change the way I determine which controller handles the request; currently every controller has a can_i_handle_this($request) type of method, that really shouldn't be there. But that something I have to think about some more before I will actually implement it.

Private reduces the options, while protected opens up the possibilities.

I feel that protected opens up the wrong possibilities; because if someone needs access to that data I don't want them to force using inheritance. If the data is public; you can still use inheritance without forcing any design decision upon them.

I recently did a grep for 'extends' over one of my code bases, one that I think has a great design and I only encountered it when a third party product required me to use it, or when I extend an interface such as below.

I joined a new project recently which has a full suite of tests. Guess what happens when you remove all the access stuff? Absolutely nothing All the tests still pass.

That's because they're testing behavior and not implementation (which they should); it doesn't make sense to test the state of every public variable because variables are not interesting (really, they're not). My solution to this is to make them private since nobody cares about them anyway, while yours it to keep them public because it would add clutter. The nice thing about this is that it doesn't really matter which solution you chose if the design is good, because when you're programming against behavior (interfaces in my case, unit tests in yours), there is no way on earth that you are going to want to touch variables explicitly.

Jasper, I'm starting to like your philosophy and you are right, but because you are an experienced programmer, and you know what you are doing and why you are doing it. But imagine someone with less experience than you that reads "not using inheritance is a necessity for good design". I'm sure that they are going to close the browser, open up an editor and create the biggest God object that we've ever seen. And God objects are my worst nightmare.

Just a thought; making a variable public can screw things up, but it's also kind of annoying to write getters for every variable.

That's a flaw with PHP, just as it is with Java.

Originally Posted by Jasper Bekkers

Not using inheritance is not so much a preference as it is a necessity for good design in my opinion. In the optimal case, all polymorphism happens though an interface because it reduces coupling.

That's a very type-centric way of viewing things. In my view, there are no interfaces, polymorphism happens through objects implementing the right methods, and inheritance is used to specialize behavior.

That doesn't make sense. The same applies to methods, for the same reason. If you have a utility method in your class that is only internally used, but somewhere down the road you end up using it externally. Now later when you make a change to that internal method you will break the existing usage.

The solution to that is simple, don't break existing usage. At least with a method, the implementation can be changed. Making a data member public means you are locked into providing direct access to the value stored in memory; you can't change the implementation at all.

The solution to that is simple, don't break existing usage. At least with a method, the implementation can be changed. Making a data member public means you are locked into providing direct access to the value stored in memory; you can't change the implementation at all.

Just a thought; making a varible public can screw things up, but it's also kind of annoying to write getters for every variable.

You're not supposed to get the value for an attribute. You're supposed to get a certain state of an object. That's the point of OOP. If you wanted the value of an attribute then you should use global variables, which isn't necessarily bad, but it's bad in an OOP context. The attribute can change name or purpose at any time, but the interface remains the same, namely a getter.

Say you have a Shape, and you have a getter called getShape(), which returns the shape in question. Now, there is an attribute in there called $shape, which holds the number of sides to the shape.
Originally, the getter returned a shape with symmetrically placed corners. (think of an image)
Later we changed it to return a shape with the correct number of sides, but with randomly positioned points/corners. But if you'd called the attribute directly, you would have still received the symmetrical shape.

It's an abstract example (for your head), which can't be implemented in programming terms, but it was the first thing I thought of.

Also, the setters are more important to implement, because they may (and possibly should) contain integrity checks: Is the value a number or an array or is the string in the correct format, etc.