Support forums : The Future

We Are Doing It Wrong: References

The future of this project but not in a "I want a pony" sort of way. This is all about everything meta about Quam Plures. The general direction, the support tools, stuff like that.

Moderator: Dracones

We Are Doing It Wrong: References

Postby Tblue » Mon Jul 12, 2010 8:21 pm

I recently ran into problems with reference variables (while fixing a bug).

That made me wonder whether we really use references as we should... Turns out, we don't. References initially thought to have an impact on performance are scattered all over our code base.

Well, PHP.net says returning by reference should only be done with a good (technically valid) reason and not for performance reasons. We mainly do it for the latter. That's a problem.

Then there is passing by reference. We do that for objects and large arrays (again, for performance reasons. Or so we thought.). PHP.net doesn't say anything about that, but there is an article (PDF, click here to view online) by a PHP developer (at least I think he's one) explaining that passing values by reference does not improve performance -- on the contrary, it apparently even can have a negative impact on performance!

So, in conclusion: References should only be used when technically necessary, and not for optimization.

What does that mean for us? I think we should do as noted above: Only use references when there is a good reason -- which means we should remove references from our code where they are only used to "improve" performance. That will solve some confusing problems, like variables having unexpected values (that's what I ran into while working on the bug mentioned above).

What do you think? I will gladly do all the work as I have plenty of time on my hands right now (holidays! 5 weeks or so left).
Tblue
Dracone
 
Posts: 340
Joined: Sat Nov 21, 2009 1:35 pm
Location: Berlin, Germany

Re: We Are Doing It Wrong: References

Postby EdB » Mon Jul 12, 2010 9:25 pm

As you might be able to guess, I have no idea what you're talking about :)

BUT to answer your question: go for it. I mean, unless Yabs has some "reason" why you're just talkin' shit I say go for it. Because if I understand correctly what you're saying is there is stuff in our code that was added for an invalid reason and might actually work against us, and you want to fix it. Piece by piece, or one big shot?

Oh and I took at look at your bug fixing branch - nice. I could quibble over tiny details but overall the first few tests I did worked out well.
EdB
Dracone
User avatar
 
Posts: 2072
Joined: Sun Nov 22, 2009 7:20 am
Location: Maricopa Arizona

Re: We Are Doing It Wrong: References

Postby Tblue » Mon Jul 12, 2010 9:31 pm

Because if I understand correctly what you're saying is there is stuff in our code that was added for an invalid reason and might actually work against us, and you want to fix it.

You got that right. :)

Piece by piece, or one big shot?

I wanted to do it as one big branch, yeah.

Oh and I took at look at your bug fixing branch - nice. I could quibble over tiny details but overall the first few tests I did worked out well.

Cool. Well, I would like to clean up the reference stuff first and then R2M this branch.
Tblue
Dracone
 
Posts: 340
Joined: Sat Nov 21, 2009 1:35 pm
Location: Berlin, Germany

Re: We Are Doing It Wrong: References

Postby EdB » Tue Jul 13, 2010 5:10 am

You mean adding the reference stuff to the bug fixer? If so I'll lean all over the bug fixing portion now, so's when you got all of it in one place I'd be able to sorta know if something funky came from the reference stuff or not.
EdB
Dracone
User avatar
 
Posts: 2072
Joined: Sun Nov 22, 2009 7:20 am
Location: Maricopa Arizona

Re: We Are Doing It Wrong: References

Postby Tblue » Tue Jul 13, 2010 11:24 am

Ah, hm, I suppose I could put it into the bug fixing branch as well... But what I wanted to do is: Create a separate branch for the reference stuff, merge it and *then* merge the bug fixing branch.
Tblue
Dracone
 
Posts: 340
Joined: Sat Nov 21, 2009 1:35 pm
Location: Berlin, Germany

Re: We Are Doing It Wrong: References

Postby EdB » Tue Jul 13, 2010 4:07 pm

okay my misunderstanding. life is good :)
EdB
Dracone
User avatar
 
Posts: 2072
Joined: Sun Nov 22, 2009 7:20 am
Location: Maricopa Arizona

Re: We Are Doing It Wrong: References

Postby Yabs » Tue Jul 13, 2010 5:38 pm

Tblue wrote:there is an article (PDF, click here to view online)


Yay my choice is "enable js" or "download a pdf" :|

Great example of when references shouldn't be used :D

¥
I may have opened the door but you entered of your own free will

Image Techno Babble II
Image Tacky Pad 3
Yabs
Dracone
User avatar
 
Posts: 896
Joined: Sat Nov 21, 2009 9:59 am

Re: We Are Doing It Wrong: References

Postby Tblue » Thu Aug 25, 2011 10:29 pm

I wrote:I will gladly do all the work


Well... As you might have guessed I came to the conclusion that fixing the reference mess is not as easy as I thought: There are just too many occurrences of "reference abuse" in our code.

I propose that from now on, we only use references when there is a technical reason to use them and remove old, unneeded references when we see them.
Tblue
Dracone
 
Posts: 340
Joined: Sat Nov 21, 2009 1:35 pm
Location: Berlin, Germany

Re: We Are Doing It Wrong: References

Postby EdB » Thu Aug 25, 2011 11:17 pm

I think I sort of have a vague idea on what this is about. I've been tinkering with TemplateCache a bit lately and came across get_cache() who happens to be my friend sometimes, and sort of figured something out.
Code: Select all
if( !isset( $template ) && !empty($Blog->template_ID) )
{ // Use default template from the database
   $TemplateCache = & get_cache( 'TemplateCache' );
   $Template = & $TemplateCache->get_by_ID( $Blog->template_ID );
   $template = $Template->folder;
}
Correct me if I'm wrong, but the " & " part is what makes it " by reference ". Also, we should only do that if we really need to though personally I have no idea when or why I would need to. In all the cases I've been tinkering with lately, I get rid of the & thing in there.

That case above for example comes from _blog_main.inc.php and we never use $Template again. All we ever use is $template - the lower case version which is simply the folder name we learned. So (correct me if I'm missing it here) that means we didn't need shit by reference. All we needed was to learn the stupid folder name when we had a template ID number. And since we have a Cache of info available it makes sense to pull it from Cache instead of doing a query that might not be needed. Cuz getting it from the Cache will do the query if needed. So it makes sense, but it doesn't need to be by reference.

Not that I know WHY we would ever want to do it by reference.
EdB
Dracone
User avatar
 
Posts: 2072
Joined: Sun Nov 22, 2009 7:20 am
Location: Maricopa Arizona

Re: We Are Doing It Wrong: References

Postby EdB » Fri Aug 26, 2011 12:10 am

In other words
Code: Select all
         $TemplateCache = get_Cache( 'TemplateCache' );
         $Template = $TemplateCache->get_by_ID( $this->selected_template );
         $this->folder = $Template->folder;
works just as well, and since all I wanted to do there was get the stupid folder name there is no sense in passing by reference.

Anyway I can easily agree that we should avoid passing by reference unless we need to, but I don't really understand it so if I do it wrong lemme know :)
EdB
Dracone
User avatar
 
Posts: 2072
Joined: Sun Nov 22, 2009 7:20 am
Location: Maricopa Arizona

Next

Return to The Future

Who is online

Users browsing this forum: No registered users and 1 guest

cron