2
\$\begingroup\$

I am working on a C#.Net application that will generate an Excel report from a template. Although the code's not in the app yet, it will pull data from a SQL Server database to populate the report.

I am asking for help in reviewing my class design. My code is below, with some details of the functions removed for simplicity (if someone feels like they need to see them in order to help, I can add them).

using System;

namespace RGBBProgramSummary
{

class Program
{

    static void Main()
    {
        ReportSheet ProgramSummary = new ReportSheet("Program Summary");

        ProgramSummary.CleanUp();
    }

}
}

using System;
using System.IO;
using System.Runtime.InteropServices;
using Microsoft.Office.Interop.Excel;

namespace RGBBProgramSummary
{

class ReportBook
{
    public bool Error;
    public DateTime RunDate;
    private string FileName;

    static Application ExcelApp = new Application();
    public Workbooks reportWBs = ExcelApp.Workbooks;
    public Workbook ReportWB;

    public ReportBook()
    {
        RunDate = GetRunDate();
        FileName = GetFileName(RunDate);

        File.Copy(Properties.Settings.Default.Template, FileName);

        ReportWB = reportWBs.Open(FileName);
        //ExcelApp.Visible = true;
    }

    private DateTime GetRunDate()
    {
        ...
        return DateTime.Now.AddDays(-(int)DateTime.Now.DayOfWeek - 2);
    }

    private String GetFileName(DateTime RunDate)
    {
        string FileName;

        ...
        return Properties.Settings.Default.ReportPath + FileName;
    }

}
}

using System;
using System.Globalization;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using Microsoft.Office.Interop.Excel;

namespace RGBBProgramSummary
{
class ReportSheet : ReportBook
{
    private Worksheet reportWS;

    public ReportSheet(string SheetName)
    {
        reportWS = ReportWB.Worksheets[SheetName];

        BuildHeader(reportWS);
    }

    public void CleanUp()
    {
        ReportWB.Save();
        ReportWB.Close();

        Marshal.ReleaseComObject(reportWS);
        Marshal.ReleaseComObject(reportWBs);
        Marshal.ReleaseComObject(ReportWB);
    }

    private void BuildHeader(Worksheet WS)
    {

        int WeekColumn = 0;
        int MonthColumn = 0;

        ...
        int j = CountOfWeeks();
        ...
        List<string> MonthList = GetMonthList();
        ...
    }

    private int CountOfWeeks()
    {
        int RunDateWeek
        int FirstDateWeek

        ... 
        return RunDateWeek - FirstDateWeek + 1;
    }

    private List<string> GetMonthList()
    {

        List<string> MonthList = new List<string>();

        ...
        return MonthList;
    }

}
}

As you can see, I really only have 2 classes. ReportBook represents the Workbook itself, and ReportSheet represents the Worksheet (ReportSheet inherits from ReportBook). I'm wondering if that's overkill, and really if they should be consolidated into one class. At the least, I would need to make changes if I ever wanted to add an additional worksheet (for one, I wouldn't want to open the workbook in the constructor of the base class then).

Also, I was going create a function in the ReportSheet class to read from the aforementioned SQL DB. But, after reading answers to another question, I'm thinking that the read from the DB should be in it's own class.

\$\endgroup\$
1
\$\begingroup\$

Although arguably a Pandora's box of other complications, have you considered ditching the Interop libraries and driving this through Ace/Jet drivers via OleDB? Too often do I see well-intentioned applications fall over because Excel gets upgraded. Best-case scenario you'll have to manage multiple releases, whereas oftentimes you're stuck redeveloping because of core changes that break existing methods.

Even when I need specific functions available only to the Interop libraries, I try to avoid using an actual reference in my project. You can dynamically marshal your Excel.Application object if you're building this in .NET 4.0, and it gives you some flexibility depending on what you're trying to achieve.

If you're just interested about data transfer, I'd consider making a move to OleDB if you're comfortable with SQL-based database querying.

I think by omitting your specific methods of how the data gets into the sheet, you're leaving out one of the most important parts of the application in terms of performance. Are you dumping the data cell by cell? I'm assuming coming from SQL the data will be in DataSet/DataTable format, so you could dump to an array, and then point to an address in the sheet.

I do like that you've avoided the dreaded double-dot, and are taking the proper Marshalling release methods to make sure Excel goes away when it's supposed to.

/$.02

-- Locke

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the feedback. I have never actually heard of Ace or Jet (as bad as that might be), but went and checked them out. I'm not sure if they'll work or not, because I do really need to dump the data cell by cell. The template I'm populating is highly-formatted, with rows of data heavily mixed with rows of hard-coded values. Maybe I'm underestimating the ACE and Jet technologies though. Lastly, do you have a link that would help me find out more information with regards to your second paragraph? I'm intrigued. Thanks again. \$\endgroup\$ – Jeff B Aug 16 '13 at 13:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.