Il 23/06/2009 01:47, Jim Graham ha scritto:
Hi Mario,
The changes look safe, but I wanted to bring up the following issues:
Hi Jim,
- There are a dozen or so fields in SG2D that are commonly accessed
everywhere in the pipelines including loops. These changes introduce an
accessor for loops, but that now looks out of place with no accessors
for any of the other fields that get accessed directly. Personally the
lack of accessors hasn't bothered me, particularly since this is
performance sensitive code and accessors can sap performance by nickels
and dimes if you don't implement them correctly - to wit:
+42 :)
- Accessors can be inlined if they are final, but these aren't. As it
turns out, SG2D itself is final and so I believe that is enough for them
to be inlined, but I tend to make methods final as well to underscore
that they are intended to be inlined, and also in case we eventually
decide to make the class non-final. On the other hand, we haven't really
bothered with accessors in the first place in this code to avoid the
code bloat and the potential points of heuristic failure.
- I agree that it would be nice to ask the pipes if they need loops, I'm
not sure why your solution didn't work, but I prefer that to making them
a side effect of getTextPipe() since it makes it harder to know when the
code you are editing needs to get the loops or not, and it makes it hard
to see where they come from when editing the many validatePipe() methods.
Would you mind if I divide the patch in 2 or 3 slots?
I'll give you first the accessors patch, then a patch that fixes the
issue in subject with my current approach and finally a refactorisation
so that the loops can ask wherever they want the loops validated or not,
because this is the less trivial and because I cannot work on that much
in my company time :(.
I'll try to give you something already today, hopefully.
Cheers,
Mario
--
Mario Torre, Software Developer, http://www.jroller.com/neugens/
aicas Allerton Interworks Computer Automated Systems GmbH
Haid-und-Neu-Straße 18 * D-76131 Karlsruhe * Germany
http://www.aicas.com * Tel: +49-721-663 968-44
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF
USt-Id: DE216375633, Handelsregister HRB 109481, AG Mannheim
Geschäftsführer: Dr. James J. Hunt
Please, support open standards:
http://endsoftpatents.org/
