Yes, I guess.

I think Jan's question was about passing by reference, and that this 
isn't necessary in PHP 5, so that should resolve it. Jan?

So I have to admit that I am slightly confused, mainly because my 
knowledge about php and references is rather limited. Did the answer 
from Alexandre help or can I do anything else to resolve the question?

As mentioned I forwarded the question:



One of the Horde programmers wanted to know if it would be better to

pass the parent document as a reference when creating Nodes, Elements

or Attributes (http://bugs.horde.org/ticket/?id=4062). I am not quite

certain if there is a specific reason against it so I thought I just

try to ask you for your opinion.



Answer from Alexandre Alapetite:



I am not really sure to understand your question...



1) When creating a new element, there is already a (indirect) 
reference to the parent document, as you can see on your line 155

(the "$this" reference)

http://cvs.horde.org/co.php?r=1.5&f=framework%2FDOM%2FDOM.php



     function create_element($name)

     {

         return new 
Horde_DOM_Element($this->node->createElement($name), $this);

     }



On this line 155, the "$this" is referring to a Horde_DOM_Document 
which is the parent document, which inherits from

Horde_DOM_Node which contains a reference "$this->node" or to the real 
PHP5 XML DOMDocument.





2) If you mean passing the parent document "by reference", in 
opposition to "by value", this is not a good idea as objects

variables in PHP5 are already references. When making an assignment, 
there is no copy of the object. So passing the parent "by

reference" would make a pointer of pointer quite strange, error prone, 
and I do not see the interest.





3) In my library since version 1.8 I think, I have an additional 
attribute "myOwnerDocument" for php4DOMNode objects (which you

call "Horde_DOM_Node") which is a direct reference to the parent 
document and provides speed improvement

http://alexandre.alapetite.net/doc-alex/domxml-php4-php5/



See line 196 of

http://alexandre.alapetite.net/doc-alex/domxml-php4-php5/domxml-php4-to-php5.php.txt

You may want to update your code accordingly.

Thanks.
I don't see a reason against it but the original script does not do 
that. So I passed the question to the original author. I hope that 
I'll get a reply and would report back then.

Committed, thanks.



One questions though, shouldn't the document object be passed by 
reference to the dom constructors?

Ok, I didn't look at the whole file, only the patch.
Only the ones necessary for the configuration code.

Hm, attachment got lost

As mentioned in the header of DOM.php the whole file has been derived 
from this original script. Both the author and the source url are 
mentioned in there. I don't know if DOM.php was derived from a very 
early version of that original script or if the original version only 
included the functions that were necessary for horde at that time.



The current version of the script from Alexandre Alapetite contains a 
number of additional functions not provided by DOM.php at the moment. 
I did only transfer those additional functions that are needed by 
Kolab.php.
Fixed. See attachement.

Great!
Please document that including author, license, conditions etc. in the 
file header.
Yes, with one exception: if-clauses must always use curly braces.

Second half of the patch that applies to Kolab/Kolab.php.



Both new patches apply to current horde cvs.

It took a while but now I converted Kolab.php to use the Horde_DOM 
package. This is the first half of the patch which needs to be applied 
to framework/DOM/DOM.php.



It adds a number of additional functions from 
http://alexandre.alapetite.net/doc-alex/domxml-php4-php5/domxml-php4-to-php5.php.txt that are needed by 
Kolab.php.



I hope I matched the style of the horde sources.

This would require to rework the handling of xml within the Kolab 
driver. This is something I did not tackle so far. I probably won't 
have the time during the next weeks to do that. But I think it makes 
sense to keep this ticket open, since the conversion will be needed at 
some timepoint.

Are you going to update your patch?

We already have a Horde_DOM class in CVS HEAD for exactly that 
purpose. It has been split out from Horde/Config.php (in FRAMEWORK_3), 
so it should be merged to FRAMEWORK_3 so it can be utilized by 
Kolab.php. A patch for that would be great.

dom-domxml wrapper script

domxml -> dom patch

Kolab.php does not work together with php5.



There are some problems with references which can be fixed using the 
attached patch.



In addition Kolab.php uses the php4 domxml extension that has been 
retired in php5.



While it is possible to use a workaround (I will attach the necessary 
patches as comments) I guess it is not an acceptable solution to be 
included into horde. I'd be happy about a suggestion how to properly 
fix the code for horde and would correct the patch accordingly.