Skip to content
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

Merged
merged 8 commits into from
Dec 18, 2024
Merged

Map to Excel Sheet #1

merged 8 commits into from
Dec 18, 2024

Conversation

mayankanand7
Copy link
Owner

@mayankanand7 mayankanand7 commented Dec 17, 2024

Issue reference

millij#66

PR Contents

  • added method implementation for generating sheet from map of data
  • added test cases for the method

* @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);
Copy link

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);
Copy link

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++) {
Copy link

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) {
Copy link

@millij millij Dec 17, 2024

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?

Copy link

@millij millij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@mayankanand7 mayankanand7 merged commit ab9e53b into develop Dec 18, 2024
mayankanand7 added a commit that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants