https://issues.apache.org/bugzilla/show_bug.cgi?id=48954

--- Comment #2 from Vincent Hennebert <[email protected]> 2010-03-25 
11:16:34 UTC ---
(In reply to comment #1)

Hi Peter,

Thanks for your patch. I can't comment on the functionality itself, but I
noticed a few general issues:
- the new EncodingTriplet class is missing from the patch. You must svn add the
new class before creating the patch
- there are other compilation errors: a new parameter was added to
AbstractPageObject.createTagLogicalElement but the corresponding method calls
haven't been updated. Likewise for the TagLogicalElement constructor
- the method AbstractTripletStructuredObject.setFullyQualifiedName is no longer
visible
- there are some checkstyle issues in the new code

In AFPPageSetup:
- there is a setter for encoding, so that field can be made private

In AFPPageSetupElement:
- catching Exception is evil (in most cases). In this case I suppose you just
want to catch NumberFormatException

In TagLogicalElement:
- do you really want to store the encoding as an Integer? IIUC the int value is
wrapped into an Integer in AFPPageSetup and then unwrapped in
TagLogicalElement.setEncoding. It seems to me that the int value could directly
be passed around


Thanks,
Vincent

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to