这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@sdhiray7
Copy link
Contributor

@sdhiray7 sdhiray7 commented Aug 28, 2024

Added the code and tests for checking if the row is empty using the following libraries:

  1. Apache POI
  2. JExcel
  3. Fastexcel

Test cases for each of the library with a file containing a few records and an empty file.

try (FileInputStream file = new FileInputStream(fileLocation); ReadableWorkbook wb = new ReadableWorkbook(file)) {
Sheet sheet = wb.getFirstSheet();
try (Stream<Row> rows = sheet.openStream()) {
assertEquals(false, fastexcelHelper.isRowEmpty(rows.reduce((first, second) -> second).get()));
Copy link
Contributor

Choose a reason for hiding this comment

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

assertFalse ?

Copy link
Contributor

Choose a reason for hiding this comment

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

This reduce statement is a bit hard to understand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed it as per the review comments.

import org.dhatim.fastexcel.reader.Row;

public class FastexcelHelper {
public boolean isRowEmpty(Row row) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not formatted as per the standard. You're using tabs, where we should be using spaces.

public class JExcelHelper {

public boolean isRowEmpty(Cell[] row) {
if (row == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is using a mixture of tabs and spaces

import jxl.read.biff.BiffException;


public class JExcelDetectEmptyRowUnitTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be formatted as per the standard - this uses tabs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, please check.

@Before
public void loadExcelFile() throws IOException {

fileLocation = "src/main/resources/" + FILE_NAME;
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not need to be initialized this way. fileLocation always has the same value and so should be private static final String FILE_LOCATION initialized in the class' static initializer list

private static final String SHEET_NAME = "Sheet1";

@Test
public void givenExcelFileWithXLSXFormat_whenGetColumnNames_thenReturnsColumnNames() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

reformat with spaces

public void whenParsingFastExcelFile_thenDetectEmptyRow() throws IOException {
try (FileInputStream file = new FileInputStream(fileLocation); ReadableWorkbook wb = new ReadableWorkbook(file)) {
Sheet sheet = wb.getFirstSheet();
try (Stream<Row> rows = sheet.openStream()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this implementation not possible?

// assert all rows are empty
try (Stream<Row> rows = sheet.openStream()) {
    boolean isEmpty = rows.allMatch(fastexcelHelper::isRowEmpty);

    assertTrue(isEmpty);
}

// assert last row is empty
try (Stream<Row> rows = sheet.openStream()) {
    Optional<Row> last = rows.reduce((first, second) -> second);
    boolean isLastRowEmpty = last.map(fastexcelHelper::isRowEmpty).orElseThrow();
    assertTrue(isLastRowEmpty);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I added test cases for an empty file and a file with a few records but empty rows.

@lor6 lor6 merged commit 8a78211 into eugenp:master Oct 15, 2024
1 check passed
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.

3 participants