Quoting Miguel <[EMAIL PROTECTED]>: > >> * changed the implementation so that the Atom[] atoms and the String[] > >> tokens are reused ... rather than reallocated repeatedly. > > > > Yes, I saw that you were using that... but I was not sure how to do > > that when the array growed in size... actually... how does that work? > > The atom values themselves don't get reused. But the array which holds > them can be reused. Ok, but what I do not have clear yet, is what happens when doing this: - say you have an array of length 10 all with Atom set to some value - then you need to enlarge... I guess new Atom[10+enlargement] will erase the previous values, correct? - so you always need to copy them into the new array? > > Yes, I'm not very good in writing performace efficient code ... > > I really have no idea about the costs of certain types of > > operations... and therefore generally focus on easy to understand > > code... (at least for me :)... > > In general, your approach is the right one ... do what works for you. > > One warning flag is a 'new' operation which happens in a loop. The expense > is in the reclamation (garbage collecting) more than the allocation. Ack. You mentioned this before... is that the only thing I should be aware of when writing efficient code? > In this case it wasn't a big deal ... but I changed it on principle. > > >> * reduced the code size by combining a few calls > > > > Yes, saw that... looks fine. > > In each clause of the branch you were executing 3 or 4 lines which were > very similar, caluclating strings and comparing strings.length with > atomArray.length. If you find yourself repeating something 3 or 4 times > then think about pushing it down a level. The most important thing is that > it makes the code smaller, easier to understand, and easier to extend when > you have to add additional branches. Agreed... I did not see directly how I could do that better... > >> Q: If you have an atomArray with the various attributes as multi-token > >> strings, should it be an error if all of the strings are not the same > >> length? > > > > Not sure... but I think so. > > I think so too. > > The current code is resizing the atomArray if (tokenCount > atomCount) > > I think this is a mistake ... it should say something like > if (atomCount == 0) > allocate-the-atoms(tokenCount); > else if (tokenCount != atomCount) > throw-an-exception There was one minor thing complicating this setup... you don't know the number of tokens untill you parsed the first attribute... but I guess this should work... Egon
------------------------------------------------------- This SF.Net email is sponsored by: Oracle 10g Get certified on the hottest thing ever to hit the market... Oracle 10g. Take an Oracle 10g class now, and we'll give you the exam FREE. http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click _______________________________________________ Jmol-developers mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/jmol-developers
