Poor OOP practices that programmers should avoid

[Hall of Famer]

Here's my list, but anyone can add up to it. Id like to see how many we can come out with.

 

 

1. Public Properties/Fields:

For good object oriented programmers, every property/field defined in a class should be private or protected. The problem with public properties is that they completely break encapsulation and expose the object's basic implementation details to client coders. Moreover, public properties can be modified or erased accidentally, not appropriate for security reasons too. There's a reason why in C++ all properties are private by default.

 

2. Static Properties/Methods:

Static properties/methods skip the fundemental object instantiation process and introduce global scope, they are not even object oriented. In Java/C#, Static is often used with Singleton(an infamous anti-pattern) and Factory design to compensate for the lack of global variable support, but does not really make the program any better. Static properties/methods are difficult to debug as well, so use Dependency Injection as an alternative.

 

3. Too many if/switch statements:

You cannot write a program without conditional statement, which is why pure OO is difficult or impossible to achieve. However, its always a code smell if your program contains too many if or switch statement, especially the latter. In many cases, its possible to replace conditionals with polymorphism. a very good example is when you have switch statements checking for types/genres. For if statement, things may get a bit trickier, its up for the programmer to decide whether to refactor.

 

4. Long class/methods:

Long class/method usually is a code smell, it implies that your class may be a 'God class' and that your method needs refactoring. The book PHP Pro refactoring introduces many techniques for cutting down the size of a class/method, lots of them are really useful and Id recommend it to newbies. Class and method design should both follow the single-responsibility rule, otherwise it may be a sign that a class/method needs to be broken up into parts.

 

5. Inheritance Addict:

Inheritance is a great OO technique, trust me it is. It is one of the four fundemental OO concepts(the other three being encapsulation, polymorphism and composition), you cannot build a fully functional OO system without inheritance. However, sometimes inheritance can break encapsulation, while composition is usually better than inheritance. Use inheritance when there's a strict is-a relationship between one class and another, dont overdo it.

 

6. PHP Array Addict:

PHP arrays are a poweful tool, unfortunately they are not object oriented. PHP offers two object oriented way of Array handling, SplFixedArray and ArrayObject. Use for the former for numeric array and the latter for associative array/hash, it may be a good practice to extend these classes to add more functionality. The fact that PHP arrays are not object means that they are not subject for lots of useful object features such as type hinting.

 

7. Overuse of Magic Methods:

Magic methods are useful at times, the constructor itself is actually a magic method, while others like __call() and __toString() are proven to be quite helpful for a program. However, a magic method takes at least 3x more time to execute compared to a standard method, they should only be used when necessary. Sometimes the presence of magic method also indicates that the class is not well-designed in the very first place.

 

8. Overuse of call_user_func_array():

First of all, I doubt call_user_func_array() itself is even considered OOP at all. But anyway, its commonly used to invoke an object's method with array of parameters. The problem with this callback function is that, its slow, its slower than a snail. Using __call() together with call_user_func_array() makes your program 17x slower, while its much faster to simply use variable method name such as $object->$method($arg, $arg2...).

 

9. Overuse of Method chaining:

Method chaining is a very interesting and convenient feature, it is inspired by smalltalk's cascade feature. Unfortunately, method chaining is not as good and powerful as smalltalk's cascade, and abusing method chaining can easily break encapsulation and introduce tight coupling(dependency) between methods. Id personally only use it for builder object(like a query builder) or setter methods.

 

10. Procedural code:

Need not to explain much on this one, the presence of procedural code is an implification that your code is not object oriented. What to do with it? Refactor, refactor and refactor, plain and simple.

 

 

 

[Barrikor]

Not that I disagree with you on everything, but every method/practice was created for a reason.

 

Overuse of any aspect of programming is bad... the code should have a healthy balance.

[trq]

Hilarious.

[ignace]

Hilarious.

Indeed. You talk about poor OOP practices and you mention procedural code. That's like writing an article about Italy and then go on describing the country-side in Australia.

 

How call_user_func and if/switch fit into the poor OOP practices is also lost on me.

[KevinM1]

I would love to see HoF's version of Hello World.

[Hall of Famer]

Not that I disagree with you on everything, but every method/practice was created for a reason.

 

Overuse of any aspect of programming is bad... the code should have a healthy balance.

You have a point, programming is about a healthy balance. When it comes down to the usage of design patterns, Id agree with you since a software can be written with various OO patterns. Nonetheless, there are still some basic rules that dictate that certain techniques are always better than their alternatives(like OOP is more professional than procedural), while certain practices should always be avoided(like global variables should never be used in a serious project).

 

 

 

Indeed. You talk about poor OOP practices and you mention procedural code. That's like writing an article about Italy and then go on describing the country-side in Australia.

 

How call_user_func and if/switch fit into the poor OOP practices is also lost on me.

 

Yes procedural code, along with if/switch statement aint even OOP, but the point is that your code can be improved by replacing them with good OOP techniques. For this reason, they are considered code smells and poor programming practices. You can keep playing the word game and arguing that non-OOP techniques cant be considered poor OOP practices since they aint even OOP, its your choice and I dont really care. The real point is that a program consisting of procedural code is the worst nightmare for an OO programmer/OO software, if it is not clear enough for you.

 

Perhaps the title was being a bit misleading, it may sound better if I called it 'poor practices that OO programmers should avoid' or 'poor practices that programmers should avoid when designing OO software'. You seem always to be obsessed with the unimportant/minor points in a thread, not sure whether you aint paying attention or that you are trying to digress. Sorta reminded me of the time when I used a dummy user class hierachy example to point out that PHP's namespace is lacking wildcard import feature as classes in the same package grow in number, while you ended up criticizing how the user class hierachy is inappropriately designed. The question is, who cares?

 

Anyway, by mentioning procedural code I was specifically referring to programs that use objects but are actually procedural in disguise, a good example is transaction script, an Anti-pattern I identified a few months ago. The truth is that some people consider it as OOP so long as they are simply using objects, which is not true as OOP requires more than just using objects. Can this be considered poor OOP practice? Oh yeah you can always insist that its not even OOP to begin with, again its not my business.

Edited September 5, 2013 by Hall of Famer

[requinix]

You have a point, programming is about a healthy balance. When it comes down to the usage of design patterns, Id agree with you since a software can be written with various OO patterns. Nonetheless, there are still some basic rules that dictate that certain techniques are always better than their alternatives(like OOP is more professional than procedural), while certain practices should always be avoided(like global variables should never be used in a serious project).

My serious project uses a handful of global variables. Because it actually needs global variables, as in variables that are accessible globally. It's not possible to do anything else besides cover up the fact that the values are global: function with a static variable is still essentially global, something OOP and static is still essentially global.

I guess the argument I'm making is one that I would apply to most of the bullet points you had to start: just because something can be misused doesn't mean it should not be used at all.

 

The real point is that a program consisting of procedural code is the worst nightmare for an OO programmer/OO software, if it is not clear enough for you.

Eventually you'll realize that "OOP code" looks the same as "procedural code" except that sometimes the functions have weird ->s and ::s in their names.

 

Nightmarish procedural code is a nightmare to deal with, but so is nightmarish OOP code. Fact is a poor programmer who isn't familiar with the tools available will write bad code one way or another.

 

You seem always to be obsessed with the unimportant/minor points in a thread, not sure whether you aint paying attention or that you are trying to digress.

If you come in here and start telling people what they should and should not do, we will start pointing out the "unimportant" and "minor" flaws.

[ignace]

The question is, who cares?

Yes. Pretty much the entire point of this thread.

[Hall of Famer]

Yes. Pretty much the entire point of this thread.

 

Doubt you really dont care as you replied to this thread, and twice.

 

Anyway, why not get back to the topic and come out with a list of programming practices that aspiring OOP programmer should avoid or get rid of? Thats the purpose of the thread to begin with, I aint interested in arguing on the irrelevant matters anyway.

Edited September 5, 2013 by Hall of Famer

[KevinM1]

An aspiring OOP programer should be one that recognizes that OOP is merely one programing paradigm among several, with its own strengths and weaknesses, and therefore not innately superior to other paradigms. OOP is a tool that should be applied in certain circumstances rather than all circumstances. There is no such thing as a perfect script, and not everything needs to be an object.

[trq]

 

 

Nonetheless, there are still some basic rules that dictate that certain techniques are always better than their alternatives(like OOP is more professional than procedural), while certain practices should always be avoided

 

Its comments like these that make your threads so hilarious HOF.

 

Unfortunately for you though, the more you repeat these sentiments, the more watered down your opinions become and eventually, it gets to the point where they have no worth at all.

[.josh]

If you think procedural code is bad and less professional, substandard, mark of a poor programmer, etc. then you're doing it wrong. Listening to shit like this is about as hilarious as listening to fanboys harp on how awesome mac or *nix is and how much microshaft sucks.

[Stooney]

Yes procedural code, along with if/switch statement aint even OOP, but the point is that your code can be improved by replacing them with good OOP techniques.

 

I'm curious what he thinks the 'OOP relacements' for if/switch are.....

[ignace]

Replacement for If/Switch:

http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism

 

Although I mostly agree with HoF in terms of OOP practices and principles. He's wrong about pretty much everything else. Like his view about procedural programming. I don't like hard-liners.

Edited September 30, 2013 by ignace

[adoado]

8. Overuse of call_user_func_array():

First of all, I doubt call_user_func_array() itself is even considered OOP at all. But anyway, its commonly used to invoke an object's method with array of parameters. The problem with this callback function is that, its slow, its slower than a snail. Using __call() together with call_user_func_array() makes your program 17x slower, while its much faster to simply use variable method name such as $object->$method($arg, $arg2...).

 

I'm not sure how this is possibly bad OOP design. Many language uses similar techniques in very successful symphony with encapsulation, polymorphism, etc. (think function pointers in Delphi/C). It's odd to me the justification here is speed - that's not the concern. The concern is OOP.

 

Saying "X is bad OOP because it's slow" is not a justification for not being object oriented.

 

Magic methods are also successfully used in many other programming paradigms (interfaces in Delphi when writing COM/COM+ servers), which rely extensively on OOP. Again, the speed argument baffles me.