Loop through Excel files and copy correct range in a separate file

Intro: Today I have decided to make an Excel automation task with C#. This is probably the first time I am doing something like this, thus the problems are plenty.

The task: Pretty much, the idea is the following - I have 4 excel files in folder strPath. I have to loop through all of them and make a file called Report.xlsx in the same folder, with the information from those files.

The information, that I need is anything, below row 9. Thus, the first row to copy is row number 10. That is why, the first file I loop for is saved as Report, and the bMakeOnce value is changed. After the first file is looped and saved As, I start entering into the else condition. There I locate the last used row of the Excel files and I try to copy the range into the sheetReport.

The questions:
Pretty much the code works as expected. However, I was thinking of some improvements, as far as probably there should be a way to make the things neater. E.g., some fancy try-catch-finally or similar. And some good practises, e.g. what would make to a separate function?

```
using System;
using System.IO;
using Excel = Microsoft.Office.Interop.Excel;

class MainClass
{
static void Main()
{
string strPath = Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), @"..\..\..\"));
string[] strFiles = Directory.GetFiles(strPath);
Excel.Application excel = null;
bool bMakeOnce = true;
string strReportName = "Report.xlsx";
int intFirstLine = 10;
int intLastColumn = 50;
int lastRow;
int lastRowReport;
int intTotalRows;

Excel.Workbook wkbReport = null;
string strWkbReportPath;
int n = 0;

excel = new Excel.Application();
excel.Visible = true;

foreach (string strFile in strFiles)
{
if (strFile.Contains(strReportName))
{
Console.WriteLine(strReportName + " is deleted.");
File.Delete(strFile);

string strReportName = "Report.xlsx";
int intFirstLine = 10;
int intLastColumn = 50;

You don't use magic strings or numbers. I like it ;-)

bool bMakeOnce = true;
string strReportName = "Report.xlsx";
int intFirstLine = 10;

We don't use hungarian notation in C#. The variables names should be respectively

var makeOnce = false;
var reportFileName = "..";
var firstLine = 10;

and so on...

strWkbReportPath = wkb.Path + "\\" + strReportName;

You used the Path.Combine method once you can use it here too

var reportFullName = Path.Combine(path, reportFileName);

foreach (string strFile in strFiles)
{
    if (strFile.Contains(strReportName))
    {
        Console.WriteLine(strReportName + " is deleted.");
        File.Delete(strFile);
    }
}

You should separate this loop from the other one. This means instead of getting all files and deleting reports and then skipping those file names in the second loop, you first delete the reports and then process the remaining files.

This can be done by specifying a search pattern for the GetFiles method

var reportName = "Report.xlsx";
var reportFileNames = Directory.GetFiles(path, @"*{reportName}");

foreach (string reportFileName in reportFileNames)
{
    Console.WriteLine(@"{reportFileName} deleted.");
    File.Delete(reportFileName);
}

This means it'll get all files whose names end with reportName.

In order to process the remaining files you only get *.xlsx files for the next loop:

var dataFileNames = Directory.GetFiles(path, "*.xlsx");

foreach (string dataFileName in dataFileNames)
{
    ...
}

and you no longer need this condition

if (strFile.Contains(strReportName))
{
    continue;
}

bool bMakeOnce = true;

This variable is not necessary. You can make the same decition based on the report. Create if it's null otherwise skip it.

Excel.Workbook wkb = null;
Excel.Worksheet sheet = null;
Excel.Worksheet sheetReport = null;
Excel.Range rngLastReport = null;
Excel.Range rngToCopy = null;

Usually it's better to define the variables nearest to their usage. You don't need them globaly but only inse the else block so define them where you actually start using them.

In general the second loop could become

Excel.Workbook report = null;
string strWkbReportPath;
int n = 0;

excel = new Excel.Application();
excel.Visible = true;    

try
{
    var dataFileNames = Directory.GetFiles(path, "*.xlsx");

    foreach (string dataFileName in dataFileNames)
    {

        ..

        if (report == null)
        {
            var reportFullName = Path.Combine(path, reportFileName);
            wkb.SaveAs(reportFullName);
            wkb.Close();
            report = Open(excel, reportFullName);
        }
        else
        {

            var sheetReport = report.Worksheets[1];
            var sheet = wkb.Worksheets[1];
            ..

        }
    }
}
finally
{
    if (report != null)
    {
        report.Close(true);
    }
}

If you want to be sure that each workbook is properly closed you can add another try/catch for the data files inside the loop.

Now that everything is nice and clean the only thing that remains is to split the Main up in to smaller pieces.

Define constants outside the Main

const string reportName = "Report.xlsx";
const int firstLine = 10;
const int lastColumn = 50;

and encapsulate deleting old reports and creating new ones:

static void Main()
{
    ..

    DeleteReports();
    CreateReport();
}

static void DeleteReports()
{
    .. 
}

static void CreateReport()
{
    ..
}

StackExchange Code Review Q#153054, answer score: 2