Hi Jeremias,
I’d have a couple of comments regarding your changes. I’ve only had
a quick look and don’t understand this part of the code well, but
following the principle of pair review, here they are:
- at several places there is code like the following:
private void checkInTextObject() {
if (!inTextObject) {
throw new IllegalStateException("Not in text object");
}
}
I understand and don’t criticise the need for safety checks,
especially if the code is new or complex. Only, since we now have
Java 1.4 as a minimal requirement, why not use assert statements? They
were created exactly for that.
- in PDFTextUtil:
- the setTextRenderingMode(int) method is only used within
PDFTextUtil; why then not making it private? The other
setTextRenderingMode method will still be available and seems to be
much safer to use anyway.
- it seems to me that a bit more of the internal state of
a PDFTextUtil object could be controlled by the object itself,
especially in the following piece of code (PDFTextPainter, l.220):
Font f = textUtil.selectFontForChar(ch);
if (f != textUtil.getCurrentFont()) {
textUtil.writeTJ();
textUtil.setCurrentFont(f);
textUtil.writeTf(f);
textUtil.writeTextMatrix(localTransform);
}
A signalFontUsed(f) method might make sense. Basically doing the
same test as above, but inside PDFTextUtil. The code would be better
encapsulated, so more maintainable. WDYT?
- in PDFTextPainter: now that the code is working, you probably want to
remove the DEBUG variable and all the statements displaying stuff to
System.out; or replace them with proper log.debug statements ;-)
For the rest I believe when you say it’s working... Congrats!
Vincent
