Validate and import data from an Excel file

I have recently designed a module which will do bulk upload for different contents. Basically the user will upload an Excel file, I will have to read and validate headers of excel then each value of cell for all columns. I have designed a framework please validate.

Below are the vo.

Template Header details: Excel for each content is defined as template which will have an ID and will have the header (Excel column header) for each template:

public class TemplateHeaderDetails extends BaseVO {
    Integer templateId;
    String templateName;
    public String headerNames;
    String mandatory;
    Integer fieldId;

    public Integer getTemplateId() {
        return templateId;
    }

    public void setTemplateId(Integer templateId) {
        this.templateId = templateId;
    }

    public String getTemplateName() {
        return templateName;
    }

    public void setTemplateName(String templateName) {
        this.templateName = templateName;
    }

    public String getHeaderNames() {
        return headerNames;
    }

    public void setHeaderNames(String headerNames) {
        this.headerNames = headerNames;
    }

    public String getMandatory() {
        return mandatory;
    }

    public void setMandatory(String mandatory) {
        this.mandatory = mandatory;
    }

    public Integer getFieldId() {
        return fieldId;
    }

    public void setFieldId(Integer fieldId) {
        this.fieldId = fieldId;
    }

}

Each header column is given a field Id as you can see in the vo above.

We have defined validation rules:

```
public class Rule extends BaseVO {

String ruleName;
Integer ruleId;
String methodName;
String className;
String validationMessage;
List referenceFields;

public String getRuleName() {
return ruleName;
}
public void setRuleName(String ruleName) {
this.ruleName = ruleName;
}
public Integer getRuleId() {
return ruleId;
}
public void setRuleId(Integer r

Concatenating strings

Sometimes you use a StringBuilder and sometimes the compound assignment operator (+=). The key to distinguish when to use what is this: if you know the amount of concatenations then you use +=; if you don't then you use a StringBuilder (or there are too many of them).

Exceptions

You're throwing a lot of them but you don't do anything with it. That's not proper error handling.

Paths

Your path to the excel worksheet is hardcoded, changing this to something the user can select (for example by passing it to the main arguments) would be helpful.

Iterators

You have the following inside your iterators:

Iterator cells = headerRow.cellIterator();
while (cells.hasNext()) {
    Cell cell = (Cell) cells.next();

Since your Iterator is defined as Iterator, I don't think you have to cast it Cell anymore.

Typing errors

• validateTempalte

• isTempateProper

• SUCSSES

• Errro while upadating record

• getRecrodStatus

Self-defined types

In your validation methods you return something like this:

values[0]=isTempateProper;
values[1]=errorMessages;

This is very error prone (when I saw the result used, I didn't know why you suddenly casted the first value to a Boolean). Instead, create a simple class that holds those fields

class ValidationResult {
    private Boolean isTemplateProper;
    private List errorMessages;
    // Getters + Constructor
}

Subsequently this will allow you to change

if((Boolean) headerValidation[0]==false)

to

if(!validationResult.isProperTemplate())

And you will get rid of the ugly Object[] return type.

Side-effects

Printing data from inside a method that isn't supposed to is a side-effect you don't want; let methods stick to their objective. Maybe it is a remainder of debugging?

Formatting

Add more spaces and whitelines! You should never have 30 lines of code without a single whiteline between them: it feels too cramped. Also remember to put spaces between compound statements "Row no "+rowNumber and method calls appendHeaderComments(workbook,sheet,headerValidationMsg,headers);.

Magic values

In readExcel() you're hardcoding the file location ("C:\\Data.xlsx) twice. Hardcoding in itself is questionable but this is even more prone to mistakes. Define it once and use it throughout the application.

Diamond operator

Sometimes you use it, sometimes you don't. Keep in mind that this

List vMessages = new ArrayList();

can be written as

List vMessages = new ArrayList<>();

Intermediate variables

In validateRecords()'s inner loop you're declaring a variable for each rule.something() but you only use it once. That's 8 lines of code that aren't needed, definitely since you really just go from 2 words to 1 -> Very minor change.

And unless I'm looking past it: I see several variables defined there that are not used.

Looping amongst values

You're doing this a few times with different values:

headerDetail.getHeaderNames().equalsIgnoreCase("ACTION")

Consider using a collection which will allow easier handling:

String[] headersToSkip = new[] {"ACTION", "DATAID", "DATASTATUS ID", "Department Name" };

for(String header : headersToSkip) {
    if(header.equalsIgnoreCase(headerDetail.getHeaderNames())) {
        continue;
    }
}

While we're at it here: why does getHeaderNames() return a single String? Make it singular instead.

Naming

Make your methods and variables as descriptive as possible; often this means you don't abbreviate them. For example:

• assignHeaderSeqNumber => assignHeaderSequenceNumber

• params => parameters

• columnSeq => columnSequence

StackExchange Code Review Q#51181, answer score: 14