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

--- Comment #1 from Max Berger <[email protected]> 2009-10-29 06:20:57 UTC ---
Alex,

just looked at your patch:

- I like the basic idea of replacing public field with the corresponding
accessors, and making the class immutable. This is much cleaner design.

- A speed comparison would be nice - is the new implementation slower or
faster? Do you have "proof" for your claim (e.g. time render a document with
and without the patch - any significant difference?)

- As another enhancement, the flyweight pattern could be applied to minoptmax
(of course, only after your patch is applied)

BUT:
- This patch has MUCH more content than just the MinOptMax. It contains many
reformattings (probably automatically done by your IDE) AND some other renames,
cleanups, etc. Although I do believe that what you did makes sense in most
cases, this makes this patch very difficult to review.

- I also see some diffs in $Id$ lines, which should not happen (looks like
we're missing svn props here, will check that)

I know it is difficult, but could you please refafctor the patch to address
only one issue at a time? (I'll send an email about this to the fop-dev list)

-- 
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