Vincent Hennebert wrote:
Yes, the patch doesn't seem to break anything. We could even go a bit further: the cfg parameter is no longer used in the FOUserAgent.configure() method, it might be removed. Also, would the FopFactory.getUserConfig() method not be public, I would even remove it, and move the logic that is in FOUserAgent.getUserRendererConfig into FopFactory. Is that getUserConfig() method supposed to have any utility to embedding applications? I would answer no, but what do embedding specialists think about it? Vincent
I had considered all these points when developing the patch, and I whole heartedly agree with them, they are all good ones :-).. but I wanted to try and be as minimal as possible in my refactoring - at the end of the day this should just be a fairly simple, straight forward patch after all. Making these sort of changes makes committer checking more difficult and time consuming. If you are happy to review my patch file with the above changes I am *more than happy* to make them :-).
Going further along this train of thought.. IMO, in an ideal world all this config stuff should be abstracted in a separate class such as FopConfig - so code dependencies on the avalon Configurable interface would be minimised and configuration would be centrally managed.
regards, Adrian.
