debuggable

I would like to comment but I've been marked as spam, two links are too much for a comment?

Great Post!
I've always liked reading on how you breakdown conventions to really look at whether or not these concepts can hold their own weight!

In college I always asked the following question about OO that never seemed to get answered to my satisfaction:

If you have a private variable that is accessed through public or protected getter/setter methods... why is that variable private?

Through my experience I have found the access modifiers really work in only a handful of situations. You put it perfectly that a sloppy programmer will:

"[...] will do just about anything to restore power you are trying to take away from them."

The only time I can see the private keyword work is if (like in your example above) the balance variable was never meant to be written directly but still needs to be publicly read (kind of a quasi-readonly). There is no way (i think) that you can impose a "readonly" of a variable in a public context but still make it writable within the class context:

public function getBalance() {
return $this->balance;

}

public function addCredit($amount) {
$this->addTransaction($amount);

}

public function addDebit($amount) {
$this->addTransaction($amount);

}

protected final function addTransation($amount){
// do other stuff like logging or adding records

$this->balance += $amount;

}

Hindsight, you shouldn't really use PHP as a grounds to criticize OO concepts since PHP doesn't strictly impose OO rules or include many of the more advanced features that OO has to offer. It would be like criticizing the quality of dry-aged beef to your grissely McDonald's hamburger :)

@Felix: I have to disagree with you on this one. BIG TIME. You can't discard a valuable concept that has evolved over the years just because there's crappy code out there. Damn it, if we were to do that, having seen such a big amount of useless code, no single programming concept would be left!

I think Member accessibility + Interfaces + Property methods (such as those in C#) are a great concept when applied consciously, and adequately. You seem to favor 100% white boxes, that's fine. But you can't assume every bit of code in an application to expose its full API.

In fact, having worked on Java applications with millions of lines of code I can tell you Abstract Classes and Interfaces, together with effective accessibility makes the code elegant, accessible and pretty straightforward. Now I can bet you that the wrong kind of programmer would mess up with those concepts and end up producing crappy code. Just as they would using only public variables.

Anyway I could go into it in length, but just wanted to give you the general overview.

I think your examples are a good example why you should use private properties ;-)

In the first example you validate the balance as soon as it is set (and hence the balance is always valid) whereas in the second example it is not possible to validate the balance when it is set and so the balance can become invalid. Do you see the problem?

That would certainly make reading/using the CakePHP API a bit overwhelming to the average programmer if they had to see all the internal functions!

As a programmer, if I don't *need* to see a function, I don't *want* to see that function. I know enough to stay away from the "internals" of a black box, and if I don't, I do so at my own risk.

Most of the time, I'm the consumer of my own (crappy) code. I may not get it right the first time (or 10 times), but that shouldn't discourage me from (eventually) getting it right. Seeing public vs. private at least tells me that I was *trying* to do the right thing. If anything, it reminds me to be careful, or to rethink (refactor) it. Using public/private speaks louder than code comments, or lack there of...hence why I write so much crappy code :)

Having said all that, your post will make me think harder about not writing such bad code. Good post!

I agree about private properties, which are more than often a bad idea because you're kind of forbidding to extend your code. Using private is like saying "Ok, I thought of all possible needs and goals for this data, so if you won't ever have to use it". That often prove wrong however.

I would not say the same for protected properties though, using getter and setters. Your base class may only have simple setters like

public function setValue($v)
{

$this->value = $v;

}

public function getValue($v)

{

return $this->value;

}

But to keep your data coherent, that's often very usefull. Imagine you use $this->value = ... everywhere in your code (and $myobject->value = ... if the property is public). Now one day your needs change, and you want to set a $this->isValueModified flag to true everytime you change the value. Despite the «overhead» brought by getters and setters, it now prove more than handy.

In fact it is just life saving.

About the overhead. After a little test, i estimate that the setter/getter method is 0.7 seconds (to add to the 0.4 seconds of the basic = operator method) for 1 million calls. I personally find it is acceptable in exchange of clean code that is easy to maintain. As numbers thrown like this don't mean anythinghere is the simple code used to benchmark:

http://hashbin.com/77K

So you can try it at home :)

"When I see more than 2-3 private / protected methods in a class I know the code I'm looking at is in poor shape."

My version of this would be "When I see more than 0 private methods/properties in a class I know the code I'm looking at is in poor shape."

I don't use CakePHP so I won't be able to show anything, but I think this is more a development practices concern than a framework specific one. The "anybody change my values" approach is i.m.h.o archaic in term of code maintenability on medium/long term. But one should choose the patterns he use depending on his own practical cases, no rule (or about) will prove 100% right in 100% cases.

Cheers, thanks for the post.

Felix Geisendörfer said: "Show me a good piece of a CakePHP application using private / protected in a way that cannot be improved upon by a refactoring as I show as an example above."

< ?php

class BankAccount {

protected $balance = 0.00;

protected $error;

public function validate() {
if (!is_numeric($this->balance)) {

$this->error = 'And I will strike down upon...';

return false;

}

return true;
}

public function save() {
if (!$this->validate()) {

return false;

}

return file_put_contents('balance.txt', $this->balance) !== false;
}

public function setBalance($balance) {
$this->balance = $balance;

}

public function getBalance() {
return $this->balance;

}

public function getError() {
return $this->error;

}

}

$Account = new BankAccount;
$Account->setBalance('756.68 blah');

if (!$Account->save()) {

echo 'error: ' . $Account->getError();

}

1.) Validating properties when the object is saved, doesn't really make sense, as it hides the place where the error occured (when the value was set). Therefore it violates the paradigm of transparent errors.
2.) Not validating properties leaves the object in an inconsistent state which might lead to inconsistent reads.

3.) The other programmer won't change the code in a way he is not supposed to. Period. Otherwise, you know, the stick ...

4.) Hiding details is one of the most fundamental paradigms in software architecture, protocol design etc. pp. Protected and private members are there to hide, well, details that are private or protected to a class. Currently we don't have "real" properties in PHP as e.g. C# has it, that's why we use setters and getters.

5.) Nobody needs to know IP to use a browser. And that's a good thing.

6.) If you want to emulate properties, use __set() and __get(). ezComponents does that e.g. Not that you really should do that, but yeah, it is a possibility.

7.) Everybody: no, don't do it that way. Thanks.

I guess one has to have a lot of experience to be as wrong as you are :D
Anyway, I'll give you just one trivial example to make private methods useful - when you're adding or changing functionality of certain class, you know you're free to do anything you want with the private ones, because nothing from outside will ever touch them.

On contrary, when you read names of public class methods, you should have a pretty solid idea about what the class does and how it should be used.

In your second 'good' BankAccount class, you're forced to save or at elast call the validates method the balance out to the filesystem before you ever know it's valid.

That works OK in a CakePHP context where you probably POST a request to /bank_account/edit_balance to change the balance and then make another request to do anything else to the bank account. But what about a context in which your instance of the BankAccount object remains in memory between specific actions taken on it?

Someone could easily write the following code:

$bankAccount->balance = 'abc';
$bankAccount->withdraw(50);

When the code subtracted 50 from 'abc,' it would set the balance var to some nonsense value. If you had used a setter with error checking, the class could have thrown an exception when you attempted to set the balance var with non-numeric data.

To implement anything with that BankAccount class the programmer either has to persist the class or call the validates function after every action they take on the object to ensure that it has valid data.

On some parts of this I agree. In general, the existence of 'protected' makes 'private' somewhat useless (with PHP, at least). Thus, I agree that private should go. Otherwise you are sorely mistaken.

"If you think you can manage programmers or enforce API policies with a few keywords of your programming language ... think again!"

I don't see how this is a useful point at all. If someone is using code they could just as well rewrite a function or remove a class entirely and none of the keywords in said code can do anything about it. Shocking! This is argumentum ad ridiculum.

"Programmers will do just about anything to restore power you are trying to take away from them."

Anecdotal evidence will not suffice. Prove it.

"The concept of private / protected properties and methods seems to be the most popular recipe for producing crappy code."

Ibid.

"When I see more than 2-3 private / protected methods in a class I know the code I'm looking at is in poor shape."

Ibid.

I think in general you've entirely missed the point with respect to the visibility keywords in object-oriented programming. In general, if someone wants to break the rules of your API, then obviously there is nothing you can do, but it is to their long-term detriment. There is a reason why one ought code to an interface instead of an implementation: implementations are inherently variable.

By exposing all properties as public to the rest of your application it lets your code become highly coupled to the internal implementation of code. High coupling is usually positively correlated with spaghetti code.

As other commenters have mentioned, your example fails insofar as the validity of an object is not guaranteed until someone calls the 'validate' function.

"Ok folks, here is the challenge: Show me a good piece of a CakePHP application using private / protected in a way that cannot be improved upon by a refactoring as I show as an example above."

You've created a straw man: your pattern only "works" for methods that need to have their data saved in the same way as your example does. Even then it fails to guarantee the validity of its data pre-saving.

Consider this counter-example: http://codepad.org/SIqOZhJZ

"Predicting what other programmers will want to do with your code is like playing number guessing with uuids."

Another straw man; you shouldn't have to think about how others will use your code. If you've written nicely decoupled code with single-purpose where your classes have clearly defined roles then you *know* what other people will be doing with your code.

If you find that the function of your code is open to interpretation then you're doing it wrong. Refactor your code such that the use cases of a class are clearly defined and obvious.

Hi Felix,

first I want to say that, to some extent, I do agree with it you. For
me, public/protected (I think private is so evil to be used) should be

a gentleman's agreement (convention).

So you can say:

> > This is how I designed it to be used, but if you think you have a valid reason to use it in a different way ("I'm smarter than you" can be a valid reason), then go ahead.

I suppose that I've an Enabling Attitude
http://www.martinfowler.com/bliki/SoftwareDevelopmentAttitude.html

Anyway, this guy have a point against setters and getters that makes
sense for me: http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html

Good Post.

hi mr.Felix
Thank you for your great article

Regards&Thanks.

Ganesan.S.R

Your example feels a bit contrived to me. If I were writing a BankAccount class, I'd try to make its public API mirror a real bank account's "API:" the bank teller. A bank teller allows a customer to perform just a handful of basic tasks on their account:

interface BankAccount
{

public function getBalance();

public function depositFrom(Wallet $wallet, $amount);

public function withdrawTo(Wallet $wallet, $amount);

public function transferTo(BankAccount $otherAccount, $amount);

public function transferFrom(BankAccount $otherAccount, $amount);

}

Each of these transactions, if valid, will affect the account's balance; however, only the teller needs to know how to actually change the balance directly, because only the teller can be trusted to do so legitimately. If the customer asks to make a $1000 deposit but only hands in a $10 bill, the teller's job is to throw an error (or maybe an alarm, if the customer is also wearing a ski mask).

The BankAccount class needs to be able to do something similar, and to do that it needs to be aware of all the key players (i.e., customers' Wallets and other BankAccounts). You can't let outside code modify the balance directly, because any change in the balance must necessarily affect the balance of some other money source as well. If it doesn't, you're either creating money from nothing, or annihilating existing money. The former is illegal, and the latter is a crying shame :)

In a nutshell...leaving $balance public makes it impossible for you to do all the necessary validation, since it makes it possible for client code to modify the balance without indicating where the money came from or went to.

Your first example is way safer when several programmers work together. I do agree that private and protected are evil some times , but your second example is wrong. As it was mentioned already , balance is not always valid. But besides that , when you will decide to have it always valid, you will break other code(which directly uses balance). There is a workaround in php with magic functions like __get and __set , but that's back to the first example. You assume other people think just like you , but that is incorrect. I consider that your second example is closer to procedural programming than to OOP.

Private/protected methods and properties are actually a wonderful thing, but not for the reasons you use as a strawman, though they must be used in a consistent and well thought out way. For example, if you had made your set() method private, child classes would still be unable to extend it. This alone shows that your understanding of private methods is lacking.

Even beyond that basic fact, the primary benefit of private and protected methods is not obfuscation. It is a method to help promote better encapsulation, class focus, and reduce interdependence between classes. All of which are good things, and MVC is a good example of this.

Randomly declaring properties and methods private is a bad idea, and it will lead to messy code. Proper use of private methods and properties will result in cleaner and more reusable code in the long run however.

No offense, but I think this is ridiculous.

Basically, it seems like you wouldn't have any problems making Model::__constructLinkedModel() or Model::__generateAssociation() public? Why would anyone want to pollute the Model object like that?

And also, in ASP.NET/C#, if you want to create a singleton class, this is how you do it:

public sealed class MySingleton
{

private MySingleton() {}

public static readonly MySingleton Instance = new MySingleton();

}

All thanks to the almighty private keyword! How would you do this without the private?

What about C# properties?

private string _Message;

public string Message
{

get

{

if (something)

doSomethingDimWittedHere();

return _Message;
}

set

{

_Message = value;

doSomeStuffHereToo();
becauseICan();

}

}

Why not just go back to procedural programming and make everything public, changeable, accessible..that must be fun, right? :-)

I have to completely disagree with you on this, and your own statements contradict your core argument. The main reason for using private/protected methods and properties is modularity and extensibility (the later less so for private). As others have used in examples above, if you need to change the behavior of a property, but you don't want to have to change everywhere it is read/written to, a getter/setter function consolidates all calls to that property to one place.

As for your self contradiction, one of your primary arguments against private functions is that "Programmers will do just about anything to restore power you are trying to take away from them." Then, in your example of good code, you take power away from programmers, making them always call your save() function to write the balance. Your theoretical programmer who will just restore whatever power you take away will simply just do the file_write_contents() directly in the code, removing any value from writing your code this way. Public/private/protected methods very clearly document what you can mess with and what you shouldn't.

I also have to ask, in what example of good code would you want to have invalid data in a variable?

Seriously.... wtf?
Private code is all about:

a) Noise reduction
b) Reducing the potential for mistakes

You seem to take it as an afront that the previous developer doesn't respect your 1337 skills or something.

The fact that what you are suggesting means that NO value can be read-only is incredulous. A single human being CANNOT hold an entire architecture in their head. Consider for a second ALL of the variables in ALL of your objects at run time. Now consider the fact that you cannot (with your method) guarantee the value of ANY of them. That means that every single use of these variables MUST have some sanity checking in them to ensure that someone hasn't foobarred your previously valid value before using it. Now try mixing in some multi-threading and watch your head explode.
Cmon, hunt that bug, who set balance to: "All your base are belong to us"? Can you tell? With such a system you'll have a hard time finding out.

Private code is about encapsulation. I make a bit, I decide on the public API and I FORGET the rest. The latter part is most important, I'm human so I can't hold the entire thing in my head. My Public API simplifies this and enables me to manage to fit more into my head because I can trust in the truth of the public API (as long as it is implemented properly and is a good API).
I know that if I call person.Birthday i'm going to get a date in the PAST.

I know that my configuration objects (in a dynamic UI for example) have been validated previously and they represent sane values.

I know that the ID of my database entity is a number as opposed to the string "Frog"

The fact that your example uses a weak type makes this all the more ridiculous. I can't even guarantee the fact that my balance is a number!!!
Seriously, WHAT is the use case for doing:

BankAccount->balance = "squirrel"

I mean, why? why? Why do you want this? Don't talk about future proofing, the most future proofed bit of code is blank sheet. Code what you need and change it later if you NEED.

Seriously dude, if you don't get this then you'd do well to give up on making big systems (with multiple devs editing the code) or you'll be stuck in code-churn as you attempt to iron out all the bugs.
Encapsulation is one of the biggest deals in OOP programming and you'd be wise to respect it.

I apologise for sounding so angry but this code you present makes me cry... :'(

Well, I am also mostly against what you said there... I think that private or protected need to exist so that we can verify that people are not infact pushing a squirrel into the machine. A teller would not take a squirrel as leagal tender, a squirrel would not fit into the slot of an ATM... so why should the code not reflect that?

Not only have I not looked at protected/private methods in this light before, but I completely agree!

I have to strongly disagree with your post, Felix. My arguments have already been made throughout other comments though. :P

Maybe you could say that methods which only process the args passed to them without changing the actual state of the class (by altering class variables) should be public since it's indeed impossible to predict when such a method can come in handy. (such as validates)

Allowing your balance to be in a temporarly inconsistent state does force programmers to call the exposed validates method before actually using the value

How can you possibly create a somewhat complex object with clean prototype with no private methods without breaking the postulate of "keeping your methods 2 screens or less"?

Think about it.

Assuming that you're not just writing code (as in reusable libraries) for yourself it definitely makes sense to use private and public properties and functions. I won't try to explain why, there's an abundance of that above, but I would like to point out two things:

1) Take responsibility
2) Document & communicate

As a bank, you have to handle your client's cash in a responsible matter. The same goes for a programmer; take care of your code-consumers bits in a responsible fashion!

As a bank, you instruct your clients how to deposit and withdraw cash. If people start throwing bags of cash through the windows, or show up with ski-masks and guns they either:
a) have not read your instructions

b) are criminals

In the former you a responsibility to communicate your terms (documentation) to your clients (or code-consumers). In the latter you have a responsibility to protect the cash (or bits) of your clients from any criminals.

Especially in the open-source business (with it's many code-consumers) it's imperative that you take responsibility for your code and document it. Not doing so is an outright act of laziness, and you don't get no cake for being lazy ;)

Hi Felix,

I must disagree with you. You are just ignoring the main purpose of encapsulation. Encapsulation not just means only packaging the data and functionalities in a capsule called object. It also ensures data integrity through out the life cycle of the objects. Just think yourself as an object (that you are), and try to make your brain (that is now private to yourself) public and I'm pretty sure your brain will be garbased in a while by lots of so called programmers who doesn't understand OOP but claims they does. I believe you also want your brain to be private for yourself.

Now, don't understand protected? Let's extend the same brain example ;-)

Just assume you have a brain tumour (I hope yiu won't have any), that can be cured with a minor surgery. But you can't make your brain public to prevent it from being further polluted. Then, how the doctor will do the surgery? Hmm, with protected access (actually it will be friend access, but in case of inheritance both access have same effect) that the object (in this case you will define) for the doctor.

More over, I believe the best way to write good codes is to restrict thyself to do wrong. You should never run with an axe while you need a small fruit cutter to divide the apple. A good OO programmer thinks well before designing the class architecture, and defines the object with all of its possible use-cases. If a non-OO programmer comes from the heaven and screams OMG, why this has been done and breaks the code with his funky logics, its not fault with the old OO programmer and neither the new heavenly one. Instead the fault is solely with the authority who let the 2nd programmer to deal with this code, if they couldn't find another OO programmer who understands why private/protected access specifiers are used, they should have shut their company off.

Lastly, I saw this kind of funky commenting tendancy only from the programmers who deals with mainly PHP, JavaScript or similar loosely typed languages, where an object structure can be changed in runtime with some simple tips of finger. I won't argue with competency of these languages and personally I just fond of both of them I've mentioned, but the problem occurs when programmers start with such loosely typed languages and eventually deals a few loosely typed objects that are supported in these languages, and think "Feeww... This is OOP, this was so easy and I knew it very well." Even sometimes, they become so confident that they try to prove the actual OOP concepts wrong with some small code examples. They even don't know that when you need to write such small codes in your application, you probably even don't need OOP as well and thats one of the main reason of PHP's success even though it wasn't supporting full-fledged OO support in it until version 5. OOP comes to rescue large projects where too many objects interact with each other in lots of use cases and scenerios where the object's state is business critical and so a full grain controll over illegal access of retrieval/modfication of those critical data.

Anyway, I am hoping you are expert enough to cut an apple with your axe, but its not acceptable to encourage other to do the same as that could lead to many unrecoverable accidents by others. And also too err is human, so better protect yourself from thyself.

Regards,
Shihan

If somebody does not understand why he is not supposed to modify the balance property - he will find ways around it.

I totally can't agree with you. A lot of my arguments are already posted but the one I'm not seeing here stressed enough follows.

"[...] nobody can mess with the balance of this account class and you don't even have to communicate the rules for using it to other programmers! [...]

Don't even have to communicate the rules? What the hell do you mean? What else does an error message at that point do? It communicates the rules for using the code.
How about say an InvalidTypeException with a message of "Invalid balance given. Values must be of type float." Now to me that communicates how to use the setter i.e. API.

In your poor example you don't communicate anything about how to use the code either. Of if so, please show us where exactly.

> Don't even have to communicate the rules? What the hell do you mean? What else does
> an error message at that point do? It communicates the rules for using the code.

> How about say an InvalidTypeException with a message of "Invalid balance given.

> Values must be of type float." Now to me that communicates how to use the setter

> i.e. API.

An exception? Exceptions are meant to be used as a mechanism for handling unexpected events (or should I say an exceptional event?) not as a replacement for error checking routines.

Why do you bother locking your door ? It's easy to break in anyway.

Visibility is not about enforcing security, it's about hiding what the API user shouldn't touch during normal usage, just like your lock prevents the passerby from entering your house a have a glass of beer in your kitchen. Of course, it's easy to circumvent (the Java reflection API even allows it out of the box) but it's the easiest way to start with encapsulation.

I feel that there are two separate arguments being made here. I agree with the majority of people making comments in that there should definitely be scope visibility on methods so that you can hide the implementation of methods. Hiding these internal methods is key to making sure that the external API of your objects remains consistent.

What I disagree with though is the idea that the API for your object is defined by its properties. You can't argue that you are hiding your object's properties when you create public getters and setters for all of them. In PHP I personally use the __get and __set methods to avoid the clutter of these pointless methods. Further comments can be found here:

http://codenaked.org/archives/7-Object-Property-Visibility-and-Magic-Methods.html

Information hiding is one of the cornerstones of OOP. Without private/protected modifiers ... that's like taking a limb off a person.