Importing an Excel document into a project

I use the following code to verify that columns have data and display the appropriate error message.

My code is working but it just doesn't look tidy. Is there any way to refactor all the if statements into a loop to display the error message, possibly another method which checks all? checkIfColumnIsEmpty is a bool method which returns true if the column is empty.

```
//if either column 0 and 1 are empty && column 2,3,4 and 5 are not
if (!checkIfColumnisEmpty(r.ItemArray[0]) || !checkIfColumnisEmpty(r.ItemArray[1])
&& checkIfColumnisEmpty(r.ItemArray[2]) && checkIfColumnisEmpty(r.ItemArray[3])
&& checkIfColumnisEmpty(r.ItemArray[4]) && checkIfColumnisEmpty(r.ItemArray[5]))
{
if (checkIfColumnisEmpty(r.ItemArray[0]) && !checkIfColumnisEmpty(r.ItemArray[1]))
{
throw new ImportBOQException("Error importing document: First column is empty");
}
else if (!checkIfColumnisEmpty(r.ItemArray[0]) && checkIfColumnisEmpty(r.ItemArray[1]))
{
throw new ImportBOQException("Error importing document: Second column is empty");
}
else if (!checkIfColumnisEmpty(r.ItemArray[0]) && !checkIfColumnisEmpty(r.ItemArray[1]))
{
//all columns are valid so...
Column0inSpreadsheet = r.ItemArray[0] as string;
Column1inSpreadsheet = r.ItemArray[1] as string;

//Other code which performs other operations, once the level as reached this far
}
}

//if column 0 and 1 are NOT empty && Either column 2,3,4 or 5 is empty
else if (checkIfColumnisEmpty(r.ItemArray[0]) && checkIfColumnisEmpty(r.ItemArray[1])
|| !checkIfColumnisEmpty(r.ItemArray[2]) || !checkIfColumnisEmpty(r.ItemArray[3])
|| !checkIfColumnisEmpty(r.ItemArray[4]) || !checkIfColumnisEmpty(r.ItemArray[5]))
{
if (checkIfColumnisEmpty(r.ItemArray[2]))
{
throw new ImportBOQException("Error importing document: Third column is em

First of all, better names than columnX would be nice. I guess the indentation is a copying-issue, otherwise this would be the next optimization. Furthermore I have the feeling that mixing the && and the || in your condition without parenthesis is not what you want to do?

col0empty=checkIfColumnisEmpty(r.ItemArray[0])
col1empty=checkIfColumnisEmpty(r.ItemArray[1])
col2empty=checkIfColumnisEmpty(r.ItemArray[2])
col3empty=checkIfColumnisEmpty(r.ItemArray[3])
col4empty=checkIfColumnisEmpty(r.ItemArray[4])
col5empty=checkIfColumnisEmpty(r.ItemArray[5])

if ((!col0empty || !col1empty) && col2empty && col3empty && col4empty && col5empty)
{
    if (col0empty && !col1empty) errorWithEmptyColumn("First");
    else if (!col0empty && col1empty) errorWithEmptyColumn("Second");
    else if (!col0empty && !col1empty)
    {
        //Code after validation
    }
}
else if ((col0empty && col1empty) || !col2empty || !col3empty || !col4empty || !col5empty)
{
    if (col2empty) errorWithEmptyColumn("Third");
    else if (col3empty) errorWithEmptyColumn("Fourth");
    else if (col4empty) errorWithEmptyColumn("Fifth");
    else if (col5empty) errorWithEmptyColumn("Sixth");
    else
    {
        //Code after validation
    }
}
else
{
    throw new ImportBOQException("Error Uploading"); //don't throw raw exceptions
}
// } one to much

private void errorWithEmptyColumn(String columnName) throws ImportBOQException
{
    throw new ImportBOQException("Error importing document: "+columnName+" column is empty");
}

For further optimization it is import to know how similar the code in the two remaining branches is?

You could also extract the condition in an variable and give them a name: i.e. rowTypeX and rowTypeY. Maybe the result is more intuitive if you move the handling of the row types in a method each:

empty=col0empty && col1empty && col2empty && col3empty && col4empty && col5empty
rowTypeX=col2empty && col3empty && col4empty && col5empty;
rowTypeY=col0empty && col1empty;
if ( empty) throw new ImportBOQException("Empty row");
else if (rowTypeX) handleRowTypeX(r);
else if (rowTypeY) handleRowTypeY(r);
else throw new ImportBOQException("Error Uploading");

StackExchange Code Review Q#20757, answer score: 4