0

I am trying to write an app that reads from an Excel file and displays information on a Windows form app. This is my code (this is taken straight from this video: https://www.youtube.com/watch?v=lsv7rAsvYuA)

Main form:

public partial class Form1 : Form
{
    int i = 1;
    public Form1()
    {
        InitializeComponent();
    }

    private void Button1_Click(object sender, EventArgs e)
    {
        Reader excel = new Reader("C:/Users/marti/Desktop/minput.xlsm", 1);
        lbMain.Items.Add(excel.ReadCell(i, 1));
        i++;
    }
}

Reader class:

class Reader
{
    string path = "";
    _Application excel = new _Excel.Application();
    Workbook wb;
    Worksheet ws;
    
    public Reader(string path, int sheet)
    {
        this.path = path;
        wb = excel.Workbooks.Open(path);
        ws = wb.Worksheets[sheet];
    }

    public string ReadCell(int row, int col)
    {
        if (ws.Cells[row, col].Value2 != null)
        {
            return ws.Cells[row, col].Value2;
        }
        else
        {
            return "";
        }
    }
}

Pressing the button freezes the app for about 5 seconds, before finally unfreezing and displaying the information in the listbox. I first thought that it would only happen on the first button click, as that's when the file is being loaded. However, it happens on every consecutive click too.

The freezing seems to be worse with larger Excel sheets.

Advice?

kiryakov
  • 145
  • 1
  • 7
  • 1
    *I first thought that it would only happen on the first button click, as that's when the file is being loaded.* -- but aren't you loading it every time? The `Reader` constructor loads the workbook, and you construct a new reader in every `Button1_Click(object sender, EventArgs e)` callback. – dbc Jul 31 '20 at 21:20
  • You're running the excel code on the main thread, which is the form, so if it freezes for 5 seconds it means the routine you're running is taking 5 seconds while blocking the thread. You can use [Task.Run](https://stackoverflow.com/questions/18013523/when-correctly-use-task-run-and-when-just-async-await) to avoid blocking the UI thread. – insane_developer Aug 01 '20 at 01:43

2 Answers2

0

I once used this kind of source code to read out data from Excel. I had the same issue, that the program was really slow.

I only needed the values from Excel (no background color and so on) so i decided to read in all data, when opening the Windows form and saving it to an array or list. It was faster. I recommend to check if the file has been changed before, when you press an "update" button.

I have never figured out the problem but I think that the Workbook object don't contain the data, the object only contains the path and each time you want to read out data, the wb object accesses the original document once.

Best regards!

Sebastian Baum
  • 365
  • 1
  • 9
0

The Excel spreadsheet format is messy, and parsing it is complex. Your program freezes for 5 seconds because it's taking 5 seconds to read the file. You need to either find a way to read the file more quickly or do it in a way that doesn't freeze your application.

For the latter, that is what the BackgroundWorker class is designed for. You'd create an instance of the BackgroundWorker class, assign an event handler that does the slow process, and then set it running in the background while you wait.

BackgroundWorker loadingWorker = new BackgroundWorker();
int i = 1;

public Form1()
{
    InitializeComponent();
    loadingWorker.DoWork += loadingWorker_DoWork;
}

private void Button1_Click(object sender, EventArgs e)
{
    loadingWorker.RunWorkerAsync();
}

private void loadingWorker_DoWork(object sender, DoWorkEventArgs e)
{
    Reader excel = new Reader("C:/Users/marti/Desktop/minput.xlsm", 1);
    lbMain.Items.Add(excel.ReadCell(i, 1));
    i++;
}

The BackgroundWorker's DoWork event handler will run asynchronously, which will prevent the application from freezing while the loading is in progress.

Steven Gann
  • 174
  • 10
  • `BackgroundWorker` is too legacy to promote it nowadays. – aepot Aug 01 '20 at 00:46
  • @aepot BackgroundWorker is in .NET Core 3.1, so it's not exactly legacy. – Steven Gann Aug 04 '20 at 16:34
  • There's a lot of legacy classes in .NET Core added there for compartibility. Consider [TPL](https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/task-parallel-library-tpl) for asynchronous jobs. Also opening/closing Excel for each cell is not a way to go. – aepot Aug 04 '20 at 16:58
  • The spreadsheet is open only once, when the button is pressed, not for every cell. – Steven Gann Aug 04 '20 at 21:23
  • How many cells `Reader` will read on the Button click? – aepot Aug 04 '20 at 21:25
  • All of them, as far as I can tell. – Steven Gann Aug 05 '20 at 16:23
  • Nope but only single cell by the code you provided and the `Reader` class published in the question. `RunWorkerAsync()` will open file, read single cell, and nothing more. – aepot Aug 05 '20 at 16:58
  • Let me clarify: you're posting code here not only for OP but for everyone who using SO as knowledge base while googling for an answer for some random question. You probably may not just answer but also suggest best way to improve the code. I know, you're using SO for the same purpose. Just keep it in mind while asking/answering. What if someone will post here the answer of similar quality and you will use it and sink in bugs then. Probably a not good perspective. Btw, ok, It's up to you. Nevermind. – aepot Aug 05 '20 at 19:31
  • Incorrect. The Reader reads the whole file. The application code only operates on a single cell. More importantly, it only does this once when the button is pressed, not for every cell. I did this because that's how OP designed their application, and I only resolved the freezing issue. The overall architecture is outside the question's scope. – Steven Gann Aug 05 '20 at 19:32
  • Correct, but I'll leave the comment above unchanged. – aepot Aug 05 '20 at 19:33
  • No, it isn't the purpose of StackOverflow to tell people to rearchitecture their whole application when they ask a specific question. Sure, OP should be using MVVM and databinding and loading the file into a DataGrid and editing it that way, but that's making a lot of assumptions that we can't really make from the small section of code OP provided. Maybe OP should replace the spreadsheet with a MySQL DB. – Steven Gann Aug 05 '20 at 19:34
  • Finally not to suggest `BackgroundWorker` in 2020 year. If you want to suggest it, you may refer to [existing answer](https://stackoverflow.com/a/6481328/12888024) then. Omg, 9 years-old answer but still actual. – aepot Aug 05 '20 at 19:39
  • One more thing (sorry): Excel is COM object. It's highly not recommended to create and use it in different Threads. Your solution probably won't work properly. But I'm not sure here, maybe I'm wrong...That's why I still not posted my own answer. Because the guaranteed to work solution may be some Producer/Consumer pattern implementation. – aepot Aug 05 '20 at 19:47