3
\$\begingroup\$

Description

A WinForms application has a function to export objects of the following type, in various formats:

class Item
{
    public int id { get; set; }
    public string description { get; set; }
}

At the click of a button in a window, a SaveFileDialog is shown, and currently it provides the option to save the data in .txt, .csv or .xlsx format. Since there are sometimes hundreds or thousands of objects, and the UI should not freeze up, a Task is used to run this operation. This implementation works, but could be improved.

Code

public partial class ExportWindow : Form
{
    // objects to be exported
    List<Item> items;

    // event handler for the "Export" button click
    private async void exportButton_click(object sender, System.EventArgs e)
    {
        SaveFileDialog exportDialog = new SaveFileDialog();
        exportDialog.Filter = "Text File (*.txt)|*.txt|Comma-separated values file (*.csv)|*.csv|Excel spreadsheet (*.xlsx)|*.xlsx";
        exportDialog.CheckPathExists = true;
        DialogResult result = exportDialog.ShowDialog();
        if (result == DialogResult.OK)
        {
            var ext = System.IO.Path.GetExtension(saveExportFileDlg.FileName);

            try
            { 
                // update status bar
                // (it is a custom control)
                statusBar.text("Exporting");

                // now export it
                await Task.Run(() =>
                {
                    switch (ext.ToLower())
                    {
                        case ".txt":
                            saveAsTxt(exportDialog.FileName);
                            break;

                        case ".csv":
                            saveAsCsv(exportDialog.FileName);
                            break;
                    
                        case ".xlsx":
                            saveAsExcel(exportDialog.FileName);
                            break;

                        default:
                            // shouldn't happen
                            throw new Exception("Specified export format not supported.");
                    }
                });
            }
            catch (System.IO.IOException ex)
            {
                 statusBar.text("Export failed");
                 logger.logError("Export failed" + ex.Message + "\n" + ex.StackTrace);

                 return;
            }
        }
    }

    private delegate void updateProgressDelegate(int percentage);

    public void updateProgress(int percentage)
    {
        if (statusBar.InvokeRequired)
        {
            var d = updateProgressDelegate(updateProgress);
            statusBar.Invoke(d, percentage);
        }
        else
        {
            _updateProgress(percentage);
        }
    }

    private void saveAsTxt(string filename)
    {
        IProgress<int> progress = new Progress<int>(updateProgress);
        
        // save the text file, while reporting progress....
    }

    private void saveAsCsv(string filename)
    {
        IProgress<int> progress = new Progress<int>(updateProgress);
        
        using (StreamWriter writer = StreamWriter(filename))
        {
            // write the headers and the data, while reporting progres...
        }
    }

    private void saveAsExcel(string filename)
    {
        IProgress<int> progress = Progress<int>(updateProgress);

        // EPPlus magic to write the data, while reporting progress...
    }
}

Questions

How can this be refactored to make it more extensible? That is, if I wanted to add support for more file types, make it easy and quicker to modify. The switch statement could get very long. Essentially, how to comply with the Open/Closed principle?

\$\endgroup\$
5
\$\begingroup\$

I would suggest moving the actual export(s) into their own class. We can create an interface for exports. Something along the lines of

public interface IExport<T>
{
    Task SaveAsync(string fileName, IEnumerable<T> items, IProgress<int> progress = null);
    string ExportType { get; }
}

Then each export type can implement this interface.

public class ExportItemsToText : IExport<Item>
{
    public Task SaveAsync(string fileName, IEnumerable<Item> items, IProgress<int> progress = null)
    {
        throw new NotImplementedException();
    }

    public string ExportType => "txt";
}

Then in your constructor of ExportWindow

public ExportWindow(IEnumerable<IExport<Item>> exports)
{
    // if using DI otherwise could just fill in dictionary here
    ExportStrategy = exports.ToDictionary(x => x.ExportType, x => x);
}

Instead of a switch statement you can now just look up the key in the dictionary to find what export should be ran and if not found would be the same as your default case.

IExport<Item> exporter;
if (ExportStrategy.TryGetValue(ext.ToLower(), out exporter))
{
    await exporter.SaveAsync(exportDialog.FileName, items, new Progress<int>(updateProgress))
}
else
{
    throw new Exception("Specified export format not supported.");
}

Now in the future if adding support for more types you just implement the interface and update your DI container. Or if not using DI then would need to add it to the constructor of your ExportWindow.

I don't think this is a great idea but If you really don't want to create a class per export, which I think you should, you could make the dictionary IDictionary<string, Action<string>> then just put your methods in there and when adding a new type create the method and update the dictionary.

\$\endgroup\$
2
\$\begingroup\$

I just want to share what I have since I've already implemented this (sort of) in one of my previous projects (it was on ASP.NET), but it can be applied in any other environment. The implementation was similar to CharlesNRice suggestion. However, the requirement was to only have options to export system reports (which is used only one report template) to Pdf, Excel, and Word with a negotiation of having more export options in the future. So this is how I did it :

First the interface :

public interface IExportTo<T>
{
    IExportTo<T> Generate();

    void Download(string fileName);

    void SaveAs(string fileFullPath);
}

then the container class :

public class ExportTo : IDisposable
{
    private readonly IList<T> _source;

    public ExportTo(IList<T> source)
    {
        _source = source;
    }

    public ExportExcel Excel()
    {
        return new ExportExcel(_source);
    }

    public ExportPdf Pdf()
    {
        return new ExportPdf(_source);
    }
    
    public ExportWord Word()
    {
        return new ExportPdf(_source);
    }
    

    #region IDisposable

    private bool _disposed = false;

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    private void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                Dispose();
            }

            _disposed = true;
        }
    }


    ~ExportTo()
    {
        Dispose(false);
    }

    #endregion
}

I've implemented a class for each export type as we can see in the above class. I'll share one class (I'll simplify it though from the actual class).

public sealed class ExportPdf : IExportTo<T>, IDisposable
{
    private readonly IList<T> _source;

    private ExportPdf() { }

    public ExportPdf(IList<T> source) : this() => _source = source ?? throw new ArgumentNullException(nameof(source));

    public IExportTo<T> Generate()
    {
        // some implementation 
        return this;
    }

    // another overload to generate by Id 
    public IExportTo<T> Generate(long reportId)
    {
        // do some work 
        return this;
    }

    // Download report as file 
    public void Download(string fileName)
    {
       // do some work 
    }

    public void SaveAs(string fileFullPath)
    {
        throw new NotImplementedException("This function has not been implemented yet. Only download is available for now.");
    }


    #region IDisposable

    private bool _disposed = false;

    public void Dispose()
    {   
        Dispose(true);
        GC.SuppressFinalize(this);
    }


    private void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                Dispose();
            }

            _disposed = true;
        }
    }


    ~ExportPdf()
    {
        Dispose(false);
    }

    #endregion
}

Download and SaveAs are different (not the same). Download would download the exported file, while SaveAs would save the object instance. But this was implemented like this because the used dependencies.

Now usage would like this :

new ExportTo(someList)
.Pdf()
.Generate()
.Download(fileName);

This is how I've implemented in that project, it could be improved, but for business requirements it is enough.

Whenever you need to add a new type of export, just create a new sealed class, then implement IExportTo<T>, IDisposable on that class. Finally, update the container class with the new type (add a method to open a new instance of this method) and you're good to go.

\$\endgroup\$

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.