3
\$\begingroup\$

This is my first program with C# and VS and was originally a VBA Add-In I wrote. I have embarked on this as a learning exercise. The code does work and provides the desired output in a new workbook.

The code below is called from a button on a custom ribbon created in Excel. It collects data from other workbooks w/o opening them and compiles the data into a new workbook so it can be easily reviewed.

Some of the things I am looking for in the review are but not limited to:

  • Proper class design
  • Proper use of static vs non-static
  • Have best practices been followed regarding connections

I am aware that the exception handling needs improvement. Part of me thinks that warrants a question unto itself but making it more explicit is on my todo list.

Below is the only part of the code that is in the VSTO project and has been truncated.

// Ribbon Callbacks
try
{
    switch (control.Id)
    {
        case "fullReport":
            FullReport.FullReportMain(Globals.ThisAddIn.Application);
            break;
    }
}
catch (Exception ex)
{
    DialogResult dialogResult = MessageBox.Show(ex.Message, "System Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
}

The separate project in the same solution to be reviewed.

using System;
using System.Data;
using System.Data.OleDb;
using System.Globalization;
using System.Linq;
using Excel = Microsoft.Office.Interop.Excel;

namespace BfMetricsAddIn
{
    /// <summary>
    /// Static Class for FullReport button.
    /// </summary>
    public static class FullReport
    {
        private const string MNewbornsWS = "Newborns_3";

#if DEBUG

        /// <summary>
        /// Main for the FullReport.
        /// </summary>
        /// <param name="xlApp">Excel application</param>
        public static void FullReportMain(Excel.Application xlApp)
#else
        /// <summary>
        /// Main for the FullReport.
        /// </summary>
        /// <param name="xlApp">Excel Application</param>
        public static void FullReportMain(Excel.Application xlApp)
#endif
        {
#if DEBUG
            string[] pathArray = XlFileDialog.SelectFiles();
#else
            string[] pathArray = XlFileDialog.SelectFiles(xlApp);
#endif
            BreastFeedingData[] breastFeedingDataArr = new BreastFeedingData[pathArray.Length];

            for (int i = 0; i < pathArray.Length; i++)
            {
                DataTable dataTable = CreateDataTable(pathArray[i]);
                breastFeedingDataArr[i] = new BreastFeedingData(dataTable);
            }

            // Sort Array by date.
            BreastFeedingData[] sorted = breastFeedingDataArr.OrderBy(c => c.FileDate).ToArray();

            // Create a new workbook
            ReportWorkbook repWorkbook = new ReportWorkbook(xlApp, sorted);

            try
            {
                // Add data to newly created workbook
                repWorkbook.AddData();
            }
            catch (Exception)
            {
                repWorkbook.Workbook.Close(false);
                throw;
            }

            // Save new workbook
            string savePath = string.Format(
            CultureInfo.CurrentCulture, @"C:\Users\{0}\Documents\BFMetrics\ReportFiles", Environment.UserName);

            // Save in default location
            const string fileName = @"\" + "BFMetricsReport";
            repWorkbook.Workbook.SaveAs(savePath + fileName);
        }

        /// <summary>
        /// Create a DataTable from Excel workbook
        /// </summary>
        /// <param name="fileName">full path of Excel worksheet.</param>
        /// <returns>DataTable from Excel workbook.</returns>
        private static DataTable CreateDataTable(string fileName)
        {
            DataTable dt = null;
            OleDbConnection myConnection = null;
            try
            {
                string connectionString = "Provider=Microsoft.ACE.OLEDB.12.0; Data Source=" + fileName +
                    ";Extended Properties='Excel 12.0 xml;HDR=Yes;IMEX=1'";

                myConnection = new OleDbConnection(connectionString);
                myConnection.Open();

                const string sheetName = MNewbornsWS + "$";

                OleDbDataAdapter myCommand = new OleDbDataAdapter("select * from [" + sheetName + "]", myConnection);
                dt = new DataTable();
                myCommand.Fill(dt);
            }
            catch (Exception)
            {
                throw;
            }
            finally
            {
                myConnection.Close();
            }

            return dt;
        }
    }
}
using System;
using System.Globalization;
using Excel = Microsoft.Office.Interop.Excel;
using FileDialog = Microsoft.Office.Core.FileDialog;
using Office = Microsoft.Office.Core;

namespace BfMetricsAddIn
{
    /// <summary>
    /// The file dialog box selector in Excel.
    /// </summary>
    public static class XlFileDialog
    {
        /// <summary>
        /// Debugging preselected files.
        /// </summary>
        /// <returns>Predefined Array for testing.</returns>
        public static string[] SelectFiles()
        {
            string folderPath = string.Format(
                CultureInfo.CurrentCulture, @"C:\Users\{0}\Documents\BFMetrics\OriginalMonthFiles\", Environment.UserName);

            // Debugging file paths
            string[] pathArray = new string[3];
            pathArray[0] = folderPath + "BreastfeedingMetrics(Nov18).xlsx";
            pathArray[1] = folderPath + "BreastfeedingMetrics(Dec18).xlsx";
            pathArray[2] = folderPath + "BreastfeedingMetrics(Aug18).xlsx";
            return pathArray;
        }

        /// <summary>
        /// User selects files to collect data from.
        /// </summary>
        /// <param name="xlApp">Excel Application</param>
        /// <returns>Array of full file path strings.</returns>
        public static string[] SelectFiles(Excel.Application xlApp)
        {
            FileDialog dialog = xlApp.FileDialog[Office.MsoFileDialogType.msoFileDialogOpen];
            dialog.AllowMultiSelect = true;
            dialog.Filters.Add("Excel Files", "*.xlsx", 1);
            dialog.InitialFileName = string.Format(
                CultureInfo.CurrentCulture, @"C:\Users\{0}\Documents\BFMetrics\OriginalMonthFiles", Environment.UserName);

            if (dialog.Show() > 0)
            {
                string[] pathArray = new string[dialog.SelectedItems.Count];

                for (int i = 1; i < dialog.SelectedItems.Count; i++)
                {
                    pathArray[i - 1] = dialog.SelectedItems.Item(i);
                }

                if (pathArray.Length > 0)
                {
                    return pathArray;
                }

                throw new ArgumentException($"{pathArray} has a length of zero.");
            }
            else
            {
                throw new ArgumentException("File selection canceled by user.");
            }
        }
    }
}
namespace BfMetricsAddIn
{
    /// <summary>
    /// Excel workbook that will hold monthly stats report.
    /// </summary>
    public class ReportWorkbook
    {
        private readonly Excel.Application xlApp;
        private readonly BreastFeedingData[] sorted;
        private Excel.Worksheet worksheet;

        /// <summary>
        /// Initializes a new instance of the <see cref="ReportWorkbook"/> class.
        /// </summary>
        /// <param name="xlApp">An current instance of the Excel Application.</param>
        /// <param name="sorted">Array of BreastFeedingData objects sorted by date.</param>
        public ReportWorkbook(Excel.Application xlApp, BreastFeedingData[] sorted)
        {
            this.xlApp = xlApp;
            this.sorted = sorted;
            this.Workbook = this.xlApp.Workbooks.Add(Type.Missing);
        }

        /// <summary>
        /// Gets created workbook.
        /// </summary>
        public Excel.Workbook Workbook { get; }

        /// <summary>
        /// Adds the data to the workbook
        /// </summary>
        public void AddData()
        {
            this.worksheet = this.Workbook.ActiveSheet;
            this.worksheet.Name = "StatReport";
            string[] rowNames = new string[]
                { "Date", "One Hour Feeding", "Skin to Skin", "Initiation Rate", "Exclusivity Rate", "Number of Babies" };

            for (int r = 0; r < rowNames.Length; r++)
            {
                this.worksheet.Cells[r + 1, 1].Value = rowNames[r];
            }

            for (int c = 0; c < this.sorted.Length; c++)
            {
                int r = 1;
                this.worksheet.Cells[r++, c + 2].Value = "'" + this.sorted[c].FileDate.ToString("MMMyy", CultureInfo.CurrentCulture);
                this.worksheet.Cells[r++, c + 2].Value = this.sorted[c].OneHourFeeding;
                this.worksheet.Cells[r++, c + 2].Value = this.sorted[c].SkinToSkin;
                this.worksheet.Cells[r++, c + 2].Value = this.sorted[c].InitiationRate;
                this.worksheet.Cells[r++, c + 2].Value = this.sorted[c].ExclusivityRate;
                this.worksheet.Cells[r++, c + 2].Value = this.sorted[c].NumberOfNewborns;
                r = 1;
            }

            // Formatting
            Excel.Range xlDataRange = this.worksheet.Range[
                this.worksheet.Cells[1, 2], this.worksheet.Cells[6, this.sorted.Length + 2]];

            // Format doubles to percentage
            Excel.Range xlDoublesRange = this.worksheet.Range[
                this.worksheet.Cells[2, 2], this.worksheet.Cells[5, this.sorted.Length + 2]];
            xlDoublesRange.NumberFormat = "##%";

            // Set Alignment to center
            xlDataRange.HorizontalAlignment = Excel.XlHAlign.xlHAlignCenter;

            // AutoFit first column
            Excel.Range rowNameColumn = this.worksheet.Columns[1];
            rowNameColumn.EntireColumn.AutoFit();
        }
    }
}
using System;
using System.Data;

namespace BfMetricsAddIn
{
    /// <summary>
    /// One month of breast feeding data.
    /// </summary>
    public class BreastFeedingData
    {
        // Column names
        private const string MFullName = "Full Name";

        private const string MMRN = "MRN";
        private const string MOHFColumnName = "Time to First Feeding (min)";
        private const string MSTSColumnNameC = "Skin to Skin within 1 hour - Cesarean (1=Yes, 0=No)";
        private const string MSTSColumnNameV = "Skin to Skin within 1 hour - Vaginal (1=Yes, 0=No)";
        private const string MIRColumnName = "Ever Breastfed? (1=Yes, 0=No)";
        private const string MERColumnName = "Exclusive?  (1=Yes, 0=No)";
        private const string MDDColumName = "Discharge Date/Time";

        // Constants for String to search for in row
        private const string MNBCountRowName = "Mother/Infant - Count distinct";

        private const string MNBStatRowName = "Mother/Infant - Total";

        private DateTime fileDate;
        private double oneHourFeeding;
        private double skinToSkin;
        private double initiationRate;
        private double exclusivityRate;
        private int numberOfNewborns;

        /// <summary>
        /// Initializes a new instance of the <see cref="BreastFeedingData"/> class.
        /// </summary>
        /// <param name="dt">DataTable created from Excel workbook</param>
        public BreastFeedingData(DataTable dt)
        {
            this.ReadFromDataTable(dt);
        }

        /// <summary>
        /// Gets the number of newborns for the month.
        /// </summary>
        public int NumberOfNewborns
        {
            get => this.numberOfNewborns;
            internal set
            {
                if (value == 0)
                {
                    throw new ArgumentOutOfRangeException($"_numberOfNewborns = {value}");
                }
                else
                {
                    this.numberOfNewborns = value;
                }
            }
        }

        /// <summary>
        /// Gets the number of newborns fed within the first hour.
        /// </summary>
        public double OneHourFeeding
        {
            get => (double)this.oneHourFeeding / this.NumberOfNewborns;

            internal set
            {
                if (value == 0)
                {
                    throw new ArgumentOutOfRangeException($"_oneHourFeeding = {value}");
                }
                else
                {
                    this.oneHourFeeding = value;
                }
            }
        }

        /// <summary>
        /// Gets the number of newborns with skin to skin within the first hour.
        /// </summary>
        public double SkinToSkin
        {
            get => (double)this.skinToSkin / this.NumberOfNewborns;

            internal set
            {
                if (value == 0)
                {
                    throw new ArgumentOutOfRangeException($"_skinToSkin = {value}");
                }
                else
                {
                    this.skinToSkin = value;
                }
            }
        }

        /// <summary>
        /// Gets the number of newborns that have breast fed this month.
        /// </summary>
        public double InitiationRate
        {
            get => (double)this.initiationRate / this.NumberOfNewborns;

            internal set
            {
                if (value == 0)
                {
                    throw new ArgumentOutOfRangeException($"_initiationRate = {value}");
                }
                else
                {
                    this.initiationRate = value;
                }
            }
        }

        /// <summary>
        /// Gets the number of newborns that have only breast fed this month.
        /// </summary>
        public double ExclusivityRate
        {
            get => (double)this.exclusivityRate / this.NumberOfNewborns;

            internal set
            {
                if (value == 0)
                {
                    throw new ArgumentOutOfRangeException($"_exclusivityRate = {value}");
                }
                else
                {
                    this.exclusivityRate = value;
                }
            }
        }

        /// <summary>
        /// Gets the month and year associated with the data.
        /// </summary>
        public DateTime FileDate
        {
            get => this.fileDate;

            internal set
            {
                if (value == DateTime.MinValue)
                {
                    throw new ArgumentOutOfRangeException($"_fileDate = {value}");
                }
                else
                {
                    this.fileDate = value;
                }
            }
        }

        private void ReadFromDataTable(DataTable dt)
        {
            int oHFCounter = 0;
            bool firstRowFlag = false;
            bool keyValueFlagA = false;
            bool keyValueFlagB = false;

            try
            {
                foreach (DataRow row in dt.Rows)
                {
                    bool isSuccess;

                    if (!firstRowFlag)
                    {
                        string strDateTime = row[MDDColumName].ToString();
                        isSuccess = DateTime.TryParse(strDateTime, out DateTime dateTime);

                        if (isSuccess)
                        {
                            this.FileDate = dateTime;
                        }

                        firstRowFlag = true;
                    }

                    // keyValue is the first column value.
                    string keyValue = row[MFullName].ToString();

                    string oneHourFeeding = row[MOHFColumnName].ToString();
                    isSuccess = int.TryParse(oneHourFeeding, out int oHFItem);

                    if (isSuccess && oHFItem <= 60 && oHFItem > 0)
                    {
                        oHFCounter++;
                    }

                    if (keyValue.Equals(MNBCountRowName, StringComparison.Ordinal))
                    {
                        keyValueFlagA = true;

                        string numberOfNewborns = row[MMRN].ToString();
                        isSuccess = int.TryParse(numberOfNewborns, out int intNumberOfNewborns);

                        if (isSuccess)
                        {
                            this.NumberOfNewborns = intNumberOfNewborns;
                        }
                    }

                    if (keyValue.Equals(MNBStatRowName, StringComparison.Ordinal))
                    {
                        keyValueFlagB = true;

                        string s2sV = row[MSTSColumnNameV].ToString();
                        isSuccess = int.TryParse(s2sV, out int ints2sV);

                        string s2sC = row[MSTSColumnNameC].ToString();
                        isSuccess = int.TryParse(s2sV, out int ints2sC);

                        if (isSuccess)
                        {
                            this.SkinToSkin = ints2sC + ints2sV;
                        }

                        string initiationRate = row[MIRColumnName].ToString();
                        isSuccess = int.TryParse(initiationRate, out int intInitiationRate);

                        if (isSuccess)
                        {
                            this.InitiationRate = intInitiationRate;
                        }

                        string exclusivityRate = row[MERColumnName].ToString();
                        isSuccess = int.TryParse(exclusivityRate, out int intExclusivityRate);

                        if (isSuccess)
                        {
                            this.ExclusivityRate = intExclusivityRate;
                        }
                    }
                }

                if (!keyValueFlagA || !keyValueFlagB)
                {
                    throw new ArgumentException($"Both values must be true. keyValueFlagA: {keyValueFlagA} keyValueFlagB: {keyValueFlagB}.");
                }

                this.OneHourFeeding = oHFCounter;
            }
            catch (Exception)
            {
                throw;
            }
        }
    }
}
```
\$\endgroup\$
  • \$\begingroup\$ This is a nice first question! ;-) \$\endgroup\$ – t3chb0t Apr 11 at 6:54
  • \$\begingroup\$ The signature of the FullReportMain method in the FullReport class is the same in both DEBUG and else parts - is this intended? \$\endgroup\$ – t3chb0t Apr 11 at 6:56
  • \$\begingroup\$ It is not intended. I just failed to remove it XD. Good catch! \$\endgroup\$ – Courtland9777 Apr 11 at 7:02
  • 1
    \$\begingroup\$ I posted a question about an error class for VSTO and got a really good response. Hope it helps. :) codereview.stackexchange.com/q/205724/158032 \$\endgroup\$ – aduguid Apr 17 at 12:08
  • \$\begingroup\$ Also, here's my GitHub site for Excel VSTO projects github.com/Excel-projects \$\endgroup\$ – aduguid Apr 17 at 12:14

Your Answer

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

Browse other questions tagged or ask your own question.