patternjavaMinor
Creating PDF files from a ODT template
Viewed 0 times
templatecreatingodtfilesfrompdf
Problem
The code is actually a TemplateBuilder.
We have 2 different templates, French and Dutch.
We need to change the template and then convert it to PDF so it can be stored in the DB later.
The main method is
What am I looking for?
Framework used :
Code (Java 1.6) :
```
public class TemplateBuilder {
private static final Logger LOGGER = LoggerFactory.getLogger(TemplateBuilder.class);
private static final int NATIONAL_NUMBER = 0;
private static final int LANGUAGE = 2;
private static final int NAME = 3;
private static final int FIRST_NAME = 4;
private static final int STREET = 5;
private static final int ZIP = 6;
private static final int CITY = 7;
private static final int COUNTRY = 8;
private static final int BIRTH_DATE = 9;
private static final int NUMBER = 10;
private static final int EMPLOYD_BY = 12;
private static final int TYPE = 13;
private byte[] templateFR;
private byte[] templateNL;
private RarExtractor extractor;
private Report report;
/**
* The bulletin service.
*/
@Inject
@Named("bulletinWebService")
private BulletinWebService bulletinService;
/**
* The country dao.
*/
@Autowired
private CountryDAO countryDAO;
/**
* The user helper.
*/
@Autowired
private BulletinUserHelper userHelper;
private final String today = new SimpleDateFormat("yyyyMMdd").format(new Date());
/**
* Creates all the bulletins from the given templates, with
We have 2 different templates, French and Dutch.
We need to change the template and then convert it to PDF so it can be stored in the DB later.
The main method is
createBulletins().What am I looking for?
- I don't like the constantly reading the template from the
byte[]for each bulletin, so if there is a possibility this could be better (with less memory consumption) I'm fully open.
- Any other suggestions/improvements.
Framework used :
fr.opensagres.xdocreport
xdocreport
1.0.5
fr.opensagres.xdocreport
fr.opensagres.xdocreport.converter.odt.odfdom
1.0.5
fr.opensagres.xdocreport
org.odftoolkit.odfdom.converter.core
1.0.5
Code (Java 1.6) :
```
public class TemplateBuilder {
private static final Logger LOGGER = LoggerFactory.getLogger(TemplateBuilder.class);
private static final int NATIONAL_NUMBER = 0;
private static final int LANGUAGE = 2;
private static final int NAME = 3;
private static final int FIRST_NAME = 4;
private static final int STREET = 5;
private static final int ZIP = 6;
private static final int CITY = 7;
private static final int COUNTRY = 8;
private static final int BIRTH_DATE = 9;
private static final int NUMBER = 10;
private static final int EMPLOYD_BY = 12;
private static final int TYPE = 13;
private byte[] templateFR;
private byte[] templateNL;
private RarExtractor extractor;
private Report report;
/**
* The bulletin service.
*/
@Inject
@Named("bulletinWebService")
private BulletinWebService bulletinService;
/**
* The country dao.
*/
@Autowired
private CountryDAO countryDAO;
/**
* The user helper.
*/
@Autowired
private BulletinUserHelper userHelper;
private final String today = new SimpleDateFormat("yyyyMMdd").format(new Date());
/**
* Creates all the bulletins from the given templates, with
Solution
Logging multiple languages
While you do make usage of logging, you write them in sometimes in Dutch and sometimes in English.
Please try to hold to 1 language, preferable English for the logging.
Static or not?
It's clear that this String need's to hold todays day formatted.
The thing what you could improve is making the
Like this you don't have an instantiation each time you create an instantiation of the
Known memory leak
Your code has a memory leak, not by you but by Apache.
There is a tracker for it but it isn't resolved because Apache would not see it as a leak.
I came upon this because I'm the OP and the server died after approximate 250 pdf's done.
I rewrote the replace function to a simpler search, this means no looking in header and footer, and just returns the first Node what match the text to search :
Memory building up
The method
Why do you need to hold all those and return it later for saving?
What if your
Note : This is real the real situation
It's stacking up memory waiting to go out of memory.
Change the method to
You have the
While you do make usage of logging, you write them in sometimes in Dutch and sometimes in English.
Please try to hold to 1 language, preferable English for the logging.
Static or not?
private final String today = new SimpleDateFormat("yyyyMMdd").format(new Date());It's clear that this String need's to hold todays day formatted.
The thing what you could improve is making the
SimpleDateFormat static.Like this you don't have an instantiation each time you create an instantiation of the
TemplateBuilderprivate static final SimpleDateFormat DATE_FORMATTER = new SimpleDateFormat("yyyyMMdd");
private final String today = DATE_FORMATTER.format(new Date());Known memory leak
Your code has a memory leak, not by you but by Apache.
There is a tracker for it but it isn't resolved because Apache would not see it as a leak.
I came upon this because I'm the OP and the server died after approximate 250 pdf's done.
I rewrote the replace function to a simpler search, this means no looking in header and footer, and just returns the first Node what match the text to search :
private void replace(String searchField, String data, OdfTextDocument document) throws Exception {
Node node = findNode(searchField, document.getContentRoot().getChildNodes());
if (node != null) {
node.setTextContent(data);
LOGGER.info("found node : " + searchField);
} else {
LOGGER.warn("didn't found node : " + searchField);
}
}
private Node findNode(String searchField, NodeList nodeList) {
int length = nodeList.getLength();
for (int counter = 0; counter < length; counter++) {
Node node = nodeList.item(counter);
if (node.getNodeType() == Node.TEXT_NODE && searchField.equals(node.getTextContent())) {
return node;
} else {
Node child = findNode(searchField, node.getChildNodes());
if (child != null) {
return child;
}
}
}
return null;
}Memory building up
The method
public Map createBulletins() design isn't goodWhy do you need to hold all those and return it later for saving?
What if your
getCsv().getEntrySet() holds more then 30k entries?Note : This is real the real situation
It's stacking up memory waiting to go out of memory.
Change the method to
void or int if you want to return the size for a report and save every x entries. (x could also be 1). You have the
BulletinWebService in the class who provides the save so use it.Code Snippets
private final String today = new SimpleDateFormat("yyyyMMdd").format(new Date());private static final SimpleDateFormat DATE_FORMATTER = new SimpleDateFormat("yyyyMMdd");
private final String today = DATE_FORMATTER.format(new Date());private void replace(String searchField, String data, OdfTextDocument document) throws Exception {
Node node = findNode(searchField, document.getContentRoot().getChildNodes());
if (node != null) {
node.setTextContent(data);
LOGGER.info("found node : " + searchField);
} else {
LOGGER.warn("didn't found node : " + searchField);
}
}
private Node findNode(String searchField, NodeList nodeList) {
int length = nodeList.getLength();
for (int counter = 0; counter < length; counter++) {
Node node = nodeList.item(counter);
if (node.getNodeType() == Node.TEXT_NODE && searchField.equals(node.getTextContent())) {
return node;
} else {
Node child = findNode(searchField, node.getChildNodes());
if (child != null) {
return child;
}
}
}
return null;
}Context
StackExchange Code Review Q#115878, answer score: 2
Revisions (0)
No revisions yet.