Removing temporal couplings from ZUGFeRDExporter to make it more intuitive about mustangproject HOT 3 CLOSED

These look like different issues to me.

• The conformancelevel could be read from the source document, that's a good idea, feel free to implement.
• ignoreA1Errors is not meaningless because if it's used at the right place the PDFBox validity check will not take place. This check is expensive (maybe a bit, in terms of computing time) and necessary: I did receive bug reports where people tried to convert ordinary PDFs (non-archivable ones) to PDF/A-3 with my library which would be incredibly difficult to implement (we don't know any open source software except Ghostscript which could even try). So I do want those guys to get exceptions and I do want the possibility to switch those checks off for those who know what they are doing (e.g. in Gnuaccounting I export PDF/A from OpenOffice which in my experience simple don't need additional checks).
• I'm not yet sure why you introduced a factory?
• Method chaining is a nice idea, I would merge an according request. My priority, however, would be providing a alternative to the getter based API in the first place. The few calls to the exporter is only a fraction of the work which is later needed to implement IZUGFeRDExportableTransaction, IZUGFeRDExportableProduct, IZUGFeRDExportableContact, and IZUGFeRDExportableItem, a setter based API could be a good way to e.g. set defaults without having to implement null-returning methods.

from mustangproject.

I did not mean that ignoreA1Errors or setConformanceLevel is always meaningless. It is only meaningless in the snippet I posted above: The methods ignoreA1Errors and setConformanceLevel need to be called BEFORE calling PDFmakeA3compliant. If you mix up the order and call PDFmakeA3compliant first and THEN call ignoreA1Errors or setConformanceLevel, then that call is meaningless. Thus my intention was to change the API so that these methods are only available during the time it makes sense to call them (before calling PDFmakeA3compliant). They are all still there.

There are two reasons why I introduced a factory:

1. So that ignoreA1Errors or setConformanceLevel (and the like) are only available in the factory. As a result I get what I described above: The methods that configure PDF-A1 to PDF-A3 conversion behaviour are only available on the factory and because the method createFromPDFA1 returns an instance of ZUGFeRDExporter it feels intuitive that further calls on the factory will have no effect on the created exporter anymore. Also it will be impossible to call methods on the exporter (such as PDFattachZugferdFile), before the PDF was created/imported (which would also be an illegal action).
2. I wanted to simplify the code. When I first read through the class ZUGFeRDExporter I thought "oh, that is a big complicated class", because a big class has also a big context to consider. However I noticed that e.g. the methods ignoreA1Errors or setConformanceLevel do something that is only meaningful for converting PDF documents from A1 to A3. Extracting these methods into a factory results in a factory that can be read quickly and is easy to understand, because it is short and it is apparent which methods interact with which properties etc.

For the method chaining part: Great to hear that you are open to also accept a configurable implementation of IZUGFeRDExportableTransaction etc. I'd be happy to also create a pull request for that. However that would be a completely different topic then the one discussed in this ticket so far.

from mustangproject.

I merged your merge requests, I hope that solves that issue. Git still reported merge errors but eclipse merge tool didn't. In the next weeks I'll have a look if the library still works as intended :-)

from mustangproject.