1
\$\begingroup\$

I am coding a simple GUI to use with system-generated Excel files. The formatting for all of these files is all the same, however they all have multiple rows that have nothing but zeros in them, so this program is designed to open the Excel file, remove the zeros, reformat and then save them in an Output directory.

I would normally do this in VBA, but the workload requires some multithreading. At the moment it saves about 99% of the files, with it erroring out on a few of them.

I'd like to submit my code for review to see if A.) I'm designing it correctly and B.) What I'm possibly messing up that I don't even know. Also it leaves all the Excel Processes open, which I keep trying to close, but to no success.

There are only two classes: The Main form and the XLSEditor classes.

frm_Main.cs:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using Microsoft.WindowsAPICodePack;
using SGA_Distribution.Classes;
using System.IO;

namespace SGA_Distribution
{
    public partial class frm_Main : Form
    {
        private string directoryName, outputDirectory;
        string[] files;
        List<XLSEditor> xlsEditors;

        public frm_Main()
        {
            InitializeComponent();
            this.directoryName = "";
            this.lbl_FileName.Text = "Directory Name: " + this.directoryName;
        }

        private void btn_Browse_Click(object sender, EventArgs e)
        {

            using (OpenFileDialog dialog = new OpenFileDialog())
            {
                if(dialog.ShowDialog() == DialogResult.OK)
                {
                    this.directoryName = Path.GetDirectoryName(dialog.FileName);
                    this.outputDirectory = Path.GetDirectoryName(dialog.FileName) + "\\Output";
                    dialog.Dispose();
                }
            }

            this.lbl_FileName.Text = "Directory Name: " + this.directoryName;
            this.lbl_OutputDirectory.Text = "Output Directory: " + this.outputDirectory;
        }

        private void btn_Cancel_Click(object sender, EventArgs e)
        {
            this.Dispose();
        }

        private void btn_OK_Click(object sender, EventArgs e)
        {

            //Split the Directory into an array of files
            this.files = Directory.GetFiles(this.directoryName, "*.xls", SearchOption.TopDirectoryOnly);

            //Create the OutputDirectory
            if (!Directory.Exists(this.outputDirectory))
            {
                try
                {
                    Directory.CreateDirectory(this.outputDirectory);
                }
                catch (Exception ex)
                {
                    ;
                }
            }//END IF

            //Need to create an array of Tasks to use, and then create a plethora of XlSEditors to remove the zeroes, and then save them to the output directory
            MessageBox.Show(this.files.Length.ToString());


            //Load all the XLSEditors for each file in the directory to a list
            try
            {
                xlsEditors = new List<XLSEditor>();
                for (int x = 0; x < this.files.Length; x++)
                {
                    string fileName = Path.GetFileName(this.files[x]);
                    string dName = Path.GetDirectoryName(this.files[x]);
                    xlsEditors.Add(new XLSEditor(fileName, this.directoryName, this.outputDirectory));
                }//END LOAD

                int index = 0;
                Task[] taskList = new Task[xlsEditors.Count];

                foreach (XLSEditor xE in xlsEditors)
                {
                    taskList[index] = Task.Factory.StartNew(() => xE.RemoveZeros());
                    index++;
                }

                Task.WaitAll(taskList);

                MessageBox.Show("Done");
            }
            catch (Exception ex)
            {
                Console.WriteLine("Error in Tasks. " + ex.Message + " - " + ex.StackTrace);
            }

        }//END BTN_OK_CLICK


    }
}

XLSEditor.cs

using System;
using System.IO;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.Office.Interop.Excel;
using Microsoft.Office.Interop.Outlook;
using System.Windows.Forms;

namespace SGA_Distribution.Classes
{
    class XLSEditor
    {

        #region Variables
        private string _fileName, _saveAsFileName;
        private string _pdfName;
        private string _directoryName, _outputDirectory;
        private Workbook _workBook;
        private Worksheet _workSheet;
        private Microsoft.Office.Interop.Excel.Application _application;
        string[] files;

        public string FileName
        {
            get
            {
                return this._fileName;
            }
        }

        public string SaveAsFileName
        {
            get
            {
                return this._saveAsFileName;
            }
        }

        public string DirectoryName
        {
            get
            {
                return this._directoryName;
            }
        }

        public string PDFName
        {
            get
            {
                return this._pdfName;
            }
        }

        public string OutputDirectory
        {
            get
            {
                return this._outputDirectory;
            }
        }

        #endregion


        #region Constructors
        //Constructor for XLEditor Class
        public XLSEditor(string fileName)
        {
            this._fileName = fileName;
            this._application = new Microsoft.Office.Interop.Excel.Application();
        }//END CONSTRUCTOR

        public XLSEditor(string directoryName, string outputDirectory)
        {
            this._directoryName = directoryName;
            this._outputDirectory = outputDirectory;
        }//END CONSTRUCTOR 2

        public XLSEditor(string fileName, string inputDirectory, string outputDirectory)
        {
            this._fileName = inputDirectory + "\\" + fileName;
            this._saveAsFileName = outputDirectory + "\\" + fileName.Replace(".xls", " -- VERSION 2.xls");
            this._pdfName = this._saveAsFileName.Replace(".xls", ".pdf");
            this._application = new Microsoft.Office.Interop.Excel.Application();

        }//END CONSTRUCTOR 3

        #endregion


        #region Methods

        public void OpenWorkbook()
        {
            try
            {
                if (Path.GetExtension(this._fileName).Contains("xl"))
                {
                    this._workBook = this._application.Workbooks.Open(this._fileName);
                }
            }
            catch (System.Exception ex)
            {

                throw;
            }
        }//END OPENWORKBOOK

        public void OpenWorkbook(string fileName)
        {
            try
            {
                if (Path.GetExtension(fileName).Contains("xl"))
                {
                    this._workBook = this._application.Workbooks.Open(fileName);
                }
            }
            catch (System.Exception ex)
            {

                throw;
            }
        }//END OPENWORKBOOK2

        public void Exit()
        {

            if(this._workBook != null)
            {
                this._workBook.Close(false);
                System.Runtime.InteropServices.Marshal.ReleaseComObject(this._workBook);
            }

            if (this._application != null)
            {
                this._application.Quit();
                System.Runtime.InteropServices.Marshal.ReleaseComObject(this._application); 
            }

        }

        public void RemoveZeros()
        {

            //Open the Workbook
            try
            {

                Console.WriteLine("Running " + this._fileName);
                this.OpenWorkbook(this._fileName);
                this._application.EnableEvents = false;
                this._application.ScreenUpdating = false;
                this._application.Calculation = XlCalculation.xlCalculationManual;

                for (int x = 1; x <= this._workBook.Worksheets.Count; x++)
                {

                    this._workSheet = this._workBook.Worksheets[x];

                    //Set Variables for Start Row, End Row and the last column of the book
                    long AccountRow = GetSpecifiedRow(this._workSheet, "Account");
                    string EndColumn = GetColumnName(this._workSheet.Range["XFD" + AccountRow].End[XlDirection.xlToLeft].Column);
                    long EndRow = this._workSheet.Range["E" + AccountRow].End[XlDirection.xlDown].Row;

                    //Get the End Column

                    for (long i = (AccountRow + 1); i <= EndRow; i++)
                    {
                        try
                        {

                            if (i > EndRow)
                                break;

                            Console.WriteLine("Checking E" + i + " for Workbook " + this._workBook.Name + " - " + this._workSheet.Name);
                            Console.WriteLine("Value is " + this._workSheet.Range["E" + i].Value2.ToString());
                            if (IsNumeric(this._workSheet.Range["E" + i].Value2.ToString()))
                            {
                                double sumOfRow = this._application.WorksheetFunction.Sum(this._workSheet.Range["E" + i + ":" + EndColumn + i]);
                                if (sumOfRow >= 0.1)
                                {
                                    this._workSheet.Range[i + ":" + i].EntireRow.Delete(XlDeleteShiftDirection.xlShiftUp);
                                    i--;
                                    EndRow = this._workSheet.Range["E" + AccountRow].End[XlDirection.xlDown].Row;
                                }

                            }
                        }
                        catch (System.Exception e)
                        {
                            Console.WriteLine(e.Message + " - " + e.StackTrace);
                            throw;
                        }
                    }//END OF INNERLOOP


                    EndRow = this._workSheet.Range["E" + AccountRow].End[XlDirection.xlDown].Row;
                    this._workSheet.Range["A" + EndRow + ":" + EndColumn + EndRow].Borders[XlBordersIndex.xlEdgeBottom].LineStyle = XlBorderWeight.xlHairline;

                }//END OF LOOP


                this._application.EnableEvents = true;
                this._application.ScreenUpdating = true;
                this._application.Calculation = XlCalculation.xlCalculationAutomatic;

                //Close the Workbook
                this._workBook.SaveAs(this._saveAsFileName);
                this._workBook.Close(SaveChanges: false);


            }
            catch (System.Exception ex)
            {
                throw;
            }
            finally
            {
                this.Exit();
            }


        }//END REMOVEZEROS

        #endregion


        #region Functions
        public long GetSpecifiedRow(Worksheet ws, string searchRef)
        {

            long retRow = 0;
            for(int i = 1; i <= 20; i++)
            {

                try
                {
                    if(ws.Cells[i, 1].Value2.ToString() == searchRef)
                    {
                        retRow = i;
                        break;
                    }
                }catch(System.Exception e)
                {
                    ;
                }

            }//END OUTER LOOP

            return retRow;

        }//END GETSPECIFIEDROW

        public string GetColumnName(long col)
        {
            string ColumnString = "";

            decimal columnNumber = col;
            while(columnNumber > 0)
            {
                decimal currentLetterNumber = (columnNumber - 1) % 26;
                char currentLetter = (char)(currentLetterNumber + 65);
                ColumnString = currentLetter + ColumnString;
                columnNumber = (columnNumber - (currentLetterNumber + 1)) / 26;
            }

            return ColumnString;
        }//END GETCOLUMNNAME

        public bool IsNumeric(string s)
        {
            double test;
            return double.TryParse(s,out test);
        }

        #endregion
    }
}
\$\endgroup\$
1
\$\begingroup\$

Kudos on getting a working Interop application! COM can be difficult. Some issues...

  • You don't need to prefix all your member varibles with this. The underscore character is an extremely common coding convention that denotes a private member variable. this should only be used as a member qualifier when a member has the exact same name as a method parameter. Otherwise, it just clutters up your code.
  • In the following code, you are essentially calling Dispose twice. Once explicitly, and once implicitly when the using block exits.

    using (OpenFileDialog dialog = new OpenFileDialog())
    {
        if(dialog.ShowDialog() == DialogResult.OK)
        {
            this.directoryName = Path.GetDirectoryName(dialog.FileName);
            this.outputDirectory = Path.GetDirectoryName(dialog.FileName) + "\\Output";
    
            // The following line can be removed, as OpenFileDialog will be
            // disposed automatically when the using block exits.
            dialog.Dispose();
        }
    }
    
  • It may help your shutdown situation to use Marshal.FinalReleaseComObject instead of Marshal.ReleaseComObject. FinalReleaseComObject sets your reference count to zero, and ReleaseComObject only decrements it by one. It looks like you have some dangling references. That may make a good SO question.
  • Implement or override the IDisposable interface in both your classes, and do your cleanup there. If necessary, use a finalizer in combination with FinalReleaseComObject to ensure that your Excel COM references are being cleaned up correctly.
\$\endgroup\$
  • \$\begingroup\$ Thanks! Just adjusted my code to get rid of all the this. What's weird is after all of my threads are done processing, I should get a pop-up saying "Done" (which I'll remove eventually), but I don't get it \$\endgroup\$ – jDave1984 Sep 26 '17 at 17:59
  • \$\begingroup\$ Welcome! I'll take a look tonight and see if I can find anything. \$\endgroup\$ – John Stritenberger Sep 26 '17 at 18:34
  • \$\begingroup\$ Without getting down and dirty with it, I can't tell why your message box isn't firing. This might be another good SO question. \$\endgroup\$ – John Stritenberger Sep 27 '17 at 0:23
  • 1
    \$\begingroup\$ I looked into it. I was throwing an exception up the stack and that was causing the main thread to stop all other threads, so it kept waiting for all the tasks to finish \$\endgroup\$ – jDave1984 Sep 27 '17 at 13:13

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.