Read sales transactions from Excel

This is a program to read and store the data from a Excel file (.xlsx) which is uploaded by users.

• Any better approach to reduce the usage of if-else to return the error message?

• How to design or construct a good coding pattern before I code? What kind of aspects I should pay more attention on ?

Excel file uploaded by user:

| Date       | XXX | XXX | ATM | Card-AMEX | Card-MASTER | Card-VISA | Cash | Total  | By    |

| 11/01/2015 | XXX | XXX | 0   | 100       | 50.20       | 0         | 0    | 150.20 | ADMIN |    
|            | XXX | XXX | 0   | 0         | 50.00       | 0         | 0    |  50.00 | ADMIN |    
| 12/01/2015 | XXX | XXX | 0   | 200       | 10.25       | 0         | 0    | 210.25 | ADMIN |

Code

```
public SalesFileParseResults Parse_Xlsx(string filePath)
{
SalesFileParseResults res = new SalesFileParseResults();
res.Success = true;

if (File.Exists(filePath))
{
int EXPECTED_col = 15;
int lnCnt = 0;
string content = File.ReadAllText(filePath);
try
{
// Note:
// Add Reference: NOPI.dll, NOPI.OOXML.dll, NPOI.OpenXml4Net.dll
// HSSFWorkbook: xls, XSSFWorkbook: xlsx

XSSFWorkbook excel = new XSSFWorkbook(filePath);
ISheet sheet = excel.GetSheetAt(0);
if (sheet.LastRowNum > 0 && sheet.GetRow(0).LastCellNum > 0 && sheet.GetRow(0).LastCellNum -1 && endIndex > -1)
{
for (int j = startIndex; j 0)
{
res.Success = true;
}
}
else
{
res.Success = false;
res.Messages.Add("File not found.");
}

//debug
string hi = "";
foreach (SalesData item in res.SalesData)
{
hi += item.DateTime.ToString() + "\n" + item.Others + "\n" + item.CreditCardAmex + "\n" + item.CreditCardMaster + "\n" + i

The first thing that comes to mind when seeing your code is changing this

if (sheet.GetRow(i).Cells[col].ToString() == "ATM") ATM = col;
 else if (sheet.GetRow(i).Cells[col].ToString() == "Card-AMEX") AMEX = col;
 else if (sheet.GetRow(i).Cells[col].ToString() == "Card-MASTER") MASTER = col;
 else if (sheet.GetRow(i).Cells[col].ToString() == "Card-VISA") VISA = col;
 else if (sheet.GetRow(i).Cells[col].ToString() == "Cash") CASH = col;

To the following (it might seem like more lines of code, but its way more readable and doesn't repeat sheet.GetRow(i).Cells[col].ToString() 5 times.

switch(sheet.GetRow(i).Cells[col].ToString())
{
    case "ATM": ATM = col; break;
    case "Card-AMEX": AMEX = col; break;
    case "Card-MASTER": MASTER = col; break;
    case "Card-VISA": VISA = col; break;
    case "Cash": CASH = col; break;
}

Also, try not to repeat doing the same thing over and over again.

ie. you call sheet.GetRow(k) so many times. Instead create a variable and call it once and use that variable. Same concept as above and as a bonus, you will get a performance increase.

Edit

As per the question, you want a way to clean up all the if statements (the nesting)

Instead of

if(A == B)
{
    //50 lines of code
}

do

if(A != B)
{
     //return/exit/...
}

//50 lines of code

The other option to get "rid" of all the complex if logic, is to break your code blocks into their own functions/methods. I personally don't like code blocks bigger than 15 lines (incl spacing/bracers), but of course this is my opinion.

Visual studio has a really nice extension, called CodeMaid which gives you a value based on the complexity of the method. This will also teach you to create smaller, more readable, code blocks.

StackExchange Code Review Q#111881, answer score: 6