| Author |
Message
|
| klabran |
 Posted: Thu Dec 08, 2005 12:39 pm Post subject: Refactor code to move tree items... |
 |
|
Master
Joined: 19 Feb 2004
Posts: 259
Location: Flagstaff AZ
|
I am reviewing some code that moves tree items under another tree. I know I could probably write it more efficiently but I am not sure how. I have been reading on for loops and while loops with references but all of my attempts have not panned out.
I have an incoming message like
<xmlroot>
<physicalfeatures>
<feature></feature>
</physicalfeatures>
<otherphysicalfeatures>
<feature></feature>
</otherphysicalfeatures>
</xmlroot>
I need it to look like
<xmlroot>
<physicalfeatures>
<feature></feature>
<feature></feature>
</physicalfeatures>
</xmlroot>
This is my existing code that works.
| Code: |
DECLARE intPF INTEGER 1;
DECLARE intEF INTEGER CARDINALITY(refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[]);
-- Combine any Other Features
WHILE intPF <= CARDINALITY(refArrestIn.OtherPersonPhysicalCharacter.OtherPersonPhysicalCharacter_ROW[]) DO
SET refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[intEF + intPF]=(SELECT R.PhysicalFeatureCategoryText,R.PhysicalFeatureLocationText,R.PhysicalFeatureDescriptionText FROM refArrestIn.OtherPersonPhysicalCharacter.OtherPersonPhysicalCharacter_ROW[intPF] AS R);
SET intPF = intPF + 1;
END WHILE
|
I believe that perhaps a for or while loop with a reference would be better but at this point I can't seem to get anything to work.
How would you refactor this code?
Thanks,
Kevin |
|
| Back to top |
|
 |
| klabran |
| Posted: Thu Dec 08, 2005 1:27 pm Post subject: |
|
|
Master
Joined: 19 Feb 2004
Posts: 259
Location: Flagstaff AZ
|
So far I have managed to refactor it with a for loop but to be honest I actually think this is worst on clarity and I don't know if it's faster.... ?
| Code: |
declare i integer 1;
declare y integer cardinality(refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[]);
for ref as refArrestIn.OtherPersonPhysicalCharacter.OtherPersonPhysicalCharacter_ROW[] do
SET refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[y + i]=(SELECT R.PhysicalFeatureCategoryText,R.PhysicalFeatureLocationText,R.PhysicalFeatureDescriptionText FROM ref AS R);
set i=i+1;
move ref nextsibling;
end for;
|
|
|
| Back to top |
|
|
| wschutz |
| Posted: Thu Dec 08, 2005 6:18 pm Post subject: |
|
|
Jedi Knight
Joined: 02 Jun 2005
Posts: 3316
Location: IBM (retired)
|
Is this Broker v6? I think you can do it easily with xpath if its v6.
_________________
-wayne |
|
| Back to top |
|
|
| elvis_gn |
| Posted: Thu Dec 08, 2005 8:40 pm Post subject: |
|
|
Padawan
Joined: 08 Oct 2004
Posts: 1905
Location: Dubai
|
Hi klabran,
Do u have the tag feature occuring in many places in the tree.... or do u need to pick it up from only physicalfeatures and otherphysicalfeatures ...?
Are there multiple instances of physicalfeatures ...?
Also in the code u user the word physical, but I did not notice it in the XML ...
I am thinking of some logic here but I need to know the above.....Maybe your entire sample input would help.
Regards. |
|
| Back to top |
|
|
| mgk |
| Posted: Fri Dec 09, 2005 1:47 am Post subject: |
|
|
Padawan
Joined: 31 Jul 2003
Posts: 1642
|
Hi
I guess the answer depends on why you are refactoring the code. If it is for readability then you can choose anything you prefer that works  . However, if it is for speed / performance then in V5 and especially in V6 SELECT is almost always the fastest way to transform a message. So in this case I think using SELECT is fine I doubt if using references or xpath will improve its performance. This is because you will always end up executing more statements that if you used SELECT. The one thing I did notice is that your original code your cardinality function was in the condition of the while loop. Move this out to a local variable if you can, as calling cardinality each time on a tree that is not changing in the loop is a waste of time.
You could save a little time if you had a reference to PersonPhysicalDetails and used this in the SET as this could save you two navigation steps each time around your WHILE loop.
Regards,
_________________
MGK
The postings I make on this site are my own and don't necessarily represent IBM's positions, strategies or opinions.
Last edited by mgk on Fri Dec 09, 2005 7:24 am; edited 1 time in total |
|
| Back to top |
|
|
| klabran |
| Posted: Fri Dec 09, 2005 7:08 am Post subject: |
|
|
Master
Joined: 19 Feb 2004
Posts: 259
Location: Flagstaff AZ
|
| Quote: |
Is this Broker v6? I think you can do it easily with xpath if its v6.
|
This is v5 broker.
| Quote: |
Do u have the tag feature occuring in many places in the tree.... or do u need to pick it up from only physicalfeatures and otherphysicalfeatures ...?
Are there multiple instances of physicalfeatures ...?
Also in the code u user the word physical, but I did not notice it in the XML ...
|
I have the tag physicalfeatures and otherphysicalfeatures in two places in the tree and both can be 0 to many. I need to move other underneath the physical for one big tree. My sample XML was to illustrate the idea only (it does not match exactly my actual xml structure).
| Quote: |
I guess the answer depends on why you are refactoring the code.
|
I am doing it mainly for readability & a far second, speed. Also, I am doing it for learning purposes.
| Quote: |
The one thing I did notice is that your original code your cardinality function was in the condition of the while loop. Move this out to a local variable if you can, as calling cardinality each time on a tree that is not changing in the loop is a waste of time.
|
Already did it. |
|
| Back to top |
|
|
| klabran |
| Posted: Fri Dec 09, 2005 8:25 am Post subject: |
|
|
Master
Joined: 19 Feb 2004
Posts: 259
Location: Flagstaff AZ
|
So then it seems that my original code even in my haste and lack of in depth ESQL knowledge was likely the best for readability and speed.... Wow. Go Figure! That would be a first.
| Code: |
-- Physical Features
SET refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[]=(SELECT R.PhysicalFeatureCategoryText,R.PhysicalFeatureLocationText,R.PhysicalFeatureDescriptionText FROM refArrestIn.PersonPhysicalFeature.PersonPhysicalFeature_ROW[] AS R);
-- Existing and Other Feature counts
DECLARE intEF INTEGER CARDINALITY(refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[]);
DECLARE intOF INTEGER CARDINALITY(refArrestIn.OtherPersonPhysicalCharacter.OtherPersonPhysicalCharacter_ROW[]);
-- Combine Other Features with Existing Features
SET intCntr = 1;
WHILE intCntr <= intOF DO
SET refArrestOut.ActivitySuspect.PersonPhysicalDetails.PersonPhysicalFeature[intEF + intCntr] = (SELECT R.PhysicalFeatureCategoryText,R.PhysicalFeatureLocationText,R.PhysicalFeatureDescriptionText FROM refArrestIn.OtherPersonPhysicalCharacter.OtherPersonPhysicalCharacter_ROW[intCntr] AS R);
SET intCntr = intCntr + 1;
END WHILE;
|
Except for the additional need for a refence on ActivitySuspect.PersonPhysicalDetails.
On a side note... I tried to move the tree using [< + 1] and [< + intCntr] but I was suprised in both instances that this doesn't seem to do what I thought it would. AKA - [< + 1] I thought would put me at the last record index + 1 (AKA a new last row). Instead it just replaced the last row of features over and over.
[< + intCntr] basically overwrote any elements in the original tree if the "other" tree had more elements.... Hmmmm.... [< +intCntr] I thought would put me at last record index + intCntr
I thought I could do this and eliminate the need to call cardinality on the first physical feature tree (intEF). Again, I imagine the Cardinality call is better for efficiency but I am experimenting with other ways to solve this....
Thanks,
Kevin |
|
| Back to top |
|
|
|
|
