Poorly written code?

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Sphenn
Forum Commoner
Posts: 48
Joined: Sun Jul 17, 2005 8:08 pm
Location: Winnipeg, MB

Poorly written code?

Post by Sphenn »

Quick question.

Has anyone ever seen this code?

Is it just me, or is the OOP portion of it completely incorrect?

Anyway, since we're on the subject, has anyone seen any really bad code, published that is. We've all written some code that isn't great, but it's worse when it's public ;)

Sphenn
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

I would say OOP is designed to fail the speed test in that code, but it looks like perfectly okay code.

Excluding operating systems, I can tell you from experience that video games have gobs of bad code in them, regularly.
Sphenn
Forum Commoner
Posts: 48
Joined: Sun Jul 17, 2005 8:08 pm
Location: Winnipeg, MB

Post by Sphenn »

feyd wrote:I would say OOP is designed to fail the speed test in that code, but it looks like perfectly okay code.

Excluding operating systems, I can tell you from experience that video games have gobs of bad code in them, regularly.
Hmm... interesting about the games.

About the example, why are they instantiating a new class every time through the loop. Can't they just instantiate one instance, and use that?

Sphenn
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Re: Badly written code?

Post by Roja »

Sphenn wrote:Is it just me, or is the OOP portion of it completely incorrect?
Completely incorrect, no. I'd change it slightly, but its not incorrect:

Code: Select all

$testclass=new test();

for ($i=0; $i<1000000; $i++)
{

      $cnt+=$testclass->one();

}
Note that I'm moving the new class declaration out of the loop - why do a new class define on every loop? I think its done just to over-emphasize the performance hit, and prove that OOP is less efficient. I suspect that with that change, it would prove to be at least much closer to the function and procedural versions.

Nevertheless, it is an interesting idea for an article.
Sphenn wrote:Anyway, since we're on the subject, has anyone seen any really bad code, published that is. We've all written some code that isn't great, but it's worse when it's public ;)
Couple of funny ones:

http://thedailywtf.com/forums/47280/ShowPost.aspx - My favorite. :)
http://thedailywtf.com/forums/43952/ShowPost.aspx
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

technically yes, they could.. however that wouldn't really be testing OOP's overall speed for many instances. I guess it depends on how you'd be using it..
Sphenn
Forum Commoner
Posts: 48
Joined: Sun Jul 17, 2005 8:08 pm
Location: Winnipeg, MB

Post by Sphenn »

Roja, that's what I was getting at. It just doesn't seem like a true test of speed the way it's written.

Also, thanks for the articles!! :)

Sphenn
pilau
Forum Regular
Posts: 594
Joined: Sat Jul 09, 2005 10:22 am
Location: Israel

Post by pilau »

I wish I knew OOP :(
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

Sphenn wrote:Roja, that's what I was getting at. It just doesn't seem like a true test of speed the way it's written.

Also, thanks for the articles!! :)

Sphenn
I think the author used that example to show how slow using many objects can be. If you take the same example and you have an object with a couple hundred more lines of code in it then the OOP side will slowdown DRASTICALLY.

The problem is every object that is created PHP has to dynamically allocate memory for the new object. The more memory the object needs the more time it takes and more load placed on the processor.

You have to be very carefull about how you use objects in PHP. PHP has some great OOP features but none of the current versions are that efficient when it comes to creating new objects. You will find a site written completely in OOP will not be able to process close to the number of pages per second as a site written in either complete proceedurals or a combination of proceedurals and limited OOP.

If the author wanted to really test this in a better way they would have used a web stress test tool like the one from Microsoft. They would then move the

Code: Select all

$testclass=new test();
outside the loop. Then they would run a stress test on all three types of code to see how many pages per second each can generate. The OOP version would definately lag behind the others. The larger the object being created the larger the lag will be (or fewer pages per second).
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

I wonder what the figures would be like for $cnt+=test::one();
User avatar
Buddha443556
Forum Regular
Posts: 873
Joined: Fri Mar 19, 2004 1:51 pm

Post by Buddha443556 »

Jenk wrote:I wonder what the figures would be like for $cnt+=test::one();
I'm betting about the same as procedural.
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

Buddha443556 wrote:
Jenk wrote:I wonder what the figures would be like for $cnt+=test::one();
I'm betting about the same as procedural.
Not even close.

It is the same as using

Code: Select all

$cnt+=$testclass->one();
with the instance creation outside the loop.

On my system (3.2 gig P4 under XP Pro running XAMPP Server)...

$cnt+=$testclass->one();
1000000 times for 2.9005510807 seconds

Function Procedural
1000000 times for 1.00571393967 seconds

Straight Procedural
1000000 times for 0.375358819962 seconds

$cnt+=test::one();
1000000 times for 1.5131418705 seconds

OOP with class instance outside loop
1000000 times for 1.4020409584 seconds

Code: Select all

<?php
function getmicrotime() { 
    list($usec, $sec) = explode(" ",microtime()); 
    return ((float)$sec+(float)$usec); 
} 

class test{
	function one() {
	return 1;
	}
}

$t = getmicrotime();
for ($i=0; $i<1000000; $i++){
	$testclass=new test();
	$cnt+=$testclass->one();
}
$t = getmicrotime() - $t;
print "<h3> 1000000 times for $t seconds</h3>";

$t = getmicrotime();
function one(){
	return 1;
}
for ($i=0; $i<1000000; $i++) {
	$cnt+=one();
}
$t = getmicrotime() - $t;
print "<h3>1000000 times for $t seconds</h3>";

$t = getmicrotime();
for ($i=0; $i<1000000; $i++){
	$cnt+=1;
}
$t = getmicrotime() - $t;
print "<h3>1000000 times for $t seconds</h3>";

$t = getmicrotime();
for ($i=0; $i<1000000; $i++){
	$cnt+=test::one();
}
$t = getmicrotime() - $t;
print "<h3>1000000 times for $t seconds</h3>";

$t = getmicrotime();
$testclass=new test();
for ($i=0; $i<1000000; $i++){
	$cnt+=$testclass->one();
}
$t = getmicrotime() - $t;
print "<h3> 1000000 times for $t seconds</h3>";
?>
As you can see even with the $testclass=new test(); outside the loop the OOP loop is slower than either of the procedural versions.

OOP is always going to be slower doing the same thing as procedural due to how it functions. There is always more overhead when dealing with OOP code under PHP.

Oops, had to edit for a coding mistake in the $cnt+=test::one(); version as I left the $testclass=new test(); in the loop by mistake.

There is really no difference between

Code: Select all

for ($i=0; $i<1000000; $i++){
	$cnt+=test::one();
}
and

Code: Select all

$testclass=new test();
for ($i=0; $i<1000000; $i++){
	$cnt+=$testclass->one();
}
other than the first execution of test::one(); will automatically create the object.
Last edited by AKA Panama Jack on Sat Jan 14, 2006 6:53 pm, edited 4 times in total.
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Post by Roja »

Hmm. I got rather different results:

4.676577091217 Original OOP
2.7802910804749 New OOP
1.7114670276642 Functional

Thats a pretty big difference for moving that out of the loop - not at all the same.

I'm on PHP5. The source for my tests is available at the .phps, although I have source highlighting off, so you'll have to view source.

http://www.kabal-invasion.com/testing/test2.php

Discussion about methodology welcome - did I miss something?
User avatar
AKA Panama Jack
Forum Regular
Posts: 878
Joined: Mon Nov 14, 2005 4:21 pm

Post by AKA Panama Jack »

Roja wrote:Hmm. I got rather different results:

4.676577091217 Original OOP
2.7802910804749 New OOP
1.7114670276642 Functional

Thats a pretty big difference for moving that out of the loop - not at all the same.

I'm on PHP5. The source for my tests is available at the .phps, although I have source highlighting off, so you'll have to view source.

http://www.kabal-invasion.com/testing/test2.php

Discussion about methodology welcome - did I miss something?
You weren't paying attention to what was written. If you notice the test for outside the loop in my example was faster like I said it would be but it is still slower than procedural as I said it would be.

That's why I posted the code I used so others can run the same test to verify it. :)

If you want pure speed you do not use OOP. For those that say OOP is more structured they have just been brainwashed. :) Procedural can be just as structured as OOP without losing the advantage of speed.
Last edited by AKA Panama Jack on Sat Jan 14, 2006 6:57 pm, edited 1 time in total.
User avatar
Buddha443556
Forum Regular
Posts: 873
Joined: Fri Mar 19, 2004 1:51 pm

Post by Buddha443556 »

Either way still interesting results - both of you. How about declaring the method static?
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Post by Roja »

AKA Panama Jack wrote:You weren't paying attention to what was written. If you notice the test for outside the loop in my example was faster like I said it would be but it is still slower than procedural as I said it would be.
What you said was:
AKA Panama Jack wrote:Not even close. It is the same as using $cnt+=$testclass->one();
"It is the same" is quite different from "Faster like I said", yes?
AKA Panama Jack wrote:That's why I posted the code I used so others can run the same test to verify it. :)
Mine is posted as well.
AKA Panama Jack wrote:If you want pure speed you do not use OOP.
I think the results actually argue against that a bit.

With the changes moving the class declaration out of the loop, the difference between the procedural and the OOP is down to a one second difference for a million iterations. In a single web view for a user, that difference isn't even noticable.

I'll happily take a one-millionth of a second increase to be able to comfortably use OOP where it feels appropriate. Arguing otherwise makes me think of the optimization in programming rule: Premature optimization is the root of all evil. :)
Post Reply