search

qbicsoftware / offer-manager-2-portlet

The offer manager assists in creating offers with version control and person master data handling.
MIT License
2 stars 1 forks source link

Refactor OfferToPDFConverter #632

Closed jenniferboedker closed 3 years ago

jenniferboedker commented 3 years ago

Is your feature request related to a problem? Please describe. The current OfferToPDFConverter got crowded and is really hard to understand. The code uses HTML snippets and a lot of iterations to create the HTML. Especially the generation of the item table seems not to be optimal.

Describe the solution you'd like The code should be more easily readable (CleanCode), a restructuring of methods might make sense. Especially the item table generation is a lot of code and could be handled in a separate class/hiden from the rest. I think the replacement of content and the dynamic generation should be separated.

Describe alternatives you've considered A renaming of variables and the introduction of new methods that make the code more readable might also be sufficient.

Additional context It should be carefully checked what content of the template is deleted/removed. If too much of the content is removed/cleared, the styles assigned to the tags get lost and need to be hardcoded. This makes an update of the HTML template a lot of work since not only the template but a lot of code needs to be adjusted.

Steffengreiner commented 3 years ago

Nice Issue, I agree with all the points and want to add that the biggest issue is the determination on how much spacing is necessary for the dynamic addition of items per page.
Currently this is done by guess-timating how much items fit on a page(pageItemCounter and maxPageItems, but this is entirely dependent on the length of the product item descriptions and could lead to overflow into the fixed footer(which is added via css).

jenniferboedker commented 3 years ago

Idea: Use project-oriented programming

Currently, we mix the generation of the HTML code and the decision for page breaks. This leads to code with a lot of if-else statements which are hard to understand and debug or extend. The goal would be to FIRST generate the HTML content and THEN decide where the page breaks need to be set. (one of the hardest problems is to do that dynamically because of all of the edge cases that occur, ask @Steffengreiner) One could move the code for the items table to a respective class that contains all the attributes of the table e.g. each of the product groups. This could be used to explicitly check if there are items for one product group.

sven1103 commented 3 years ago

solved in #683