-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Map to Excel Sheet #1
Conversation
* @param headers a {@link List} of Header names to write in the file. <code>null</code> or empty | ||
* list will default to all writable properties. | ||
*/ | ||
void addSheet(List<Map<String, String>> rowsData, String sheetName, List<String> headers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be consistent, reorder the args
void addSheet(String sheetName, List<String> headers, List<Map<String, String>> rowsData);
// Data Rows | ||
for (int i = 0, rowNum = 1; i < rowsData.size(); i++, rowNum++) { | ||
final Row row = sheet.createRow(rowNum); | ||
final Map<String, String> rowData = rowsData.get(i); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
recommend below to hedge against null
values
final Map<String, String> rowData = rowsData.getOrDefault(i, new HashMap<>());
|
||
// Header | ||
final Row headerRow = sheet.createRow(0); | ||
for (int i = 0; i < inHeaders.size(); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use cellNo
instead of i
for consistency ?
|
||
|
||
@Override | ||
public void createTemplate(final String sheetName, final List<String> headers) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method name is ambiguous (based on the implementation)
Also, why not just use addRow
method with empty list of rows data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
… case for addSheet with rows data as List of Map
Issue reference
millij#66
PR Contents