2
\$\begingroup\$

In my .net core console application, I am reading multiple excel files and bulk copy data into a database table. It is working as it is expected, I wonder if there are any improvements to make it better. So I would be glad if you can share your comments.

Thanks in advance.

class Program
    {
        static async Task Main(string[] args)
        {
            try
            {
                string s = null;
                var d = new DirectoryInfo(@"C:\Test");
                var files = d.GetFiles("*.xlsx");
                var usersList = new List<User>();

                foreach (var file in files)
                {
                    var fileName = file.FullName;

                    using var package = new ExcelPackage(file);
                    ExcelPackage.LicenseContext = LicenseContext.NonCommercial;
                    var currentSheet = package.Workbook.Worksheets;
                    var workSheet = currentSheet.First();
                    var noOfCol = workSheet.Dimension.End.Column;
                    var noOfRow = workSheet.Dimension.End.Row;
                    for (int rowIterator = 2; rowIterator <= noOfRow; rowIterator++)
                    {
                        var user = new User
                        {
                            GameCode = workSheet.Cells[rowIterator, 1].Value?.ToString(),
                            Count = Convert.ToInt32(workSheet.Cells[rowIterator, 2].Value),
                            Email = workSheet.Cells[rowIterator, 3].Value?.ToString(),
                            Status = Convert.ToInt32(workSheet.Cells[rowIterator, 4].Value)
                        };


                        usersList.Add(user);
                    }
                }

                var conn = ConfigurationManager.ConnectionStrings["Development"].ConnectionString;
                await using var connString = new SqlConnection(conn);
                connString.Open();
                await BulkWriter.InsertAsync(usersList, "[Orders]", connString, CancellationToken.None);
            }
            catch (Exception e)
            {
                Console.WriteLine(e);
                throw;
            }
            
        }

        private class BulkWriter
        {
            private static readonly ConcurrentDictionary<Type, SqlBulkCopyColumnMapping[]> ColumnMapping =
                new ConcurrentDictionary<Type, SqlBulkCopyColumnMapping[]>();

            public static async Task InsertAsync<T>(IEnumerable<T> items, string tableName, SqlConnection connection,
                CancellationToken cancellationToken)
            {
                using var bulk = new SqlBulkCopy(connection);
                await using var reader = ObjectReader.Create(items);
                bulk.DestinationTableName = tableName;
                foreach (var colMap in GetColumnMappings<T>())
                    bulk.ColumnMappings.Add(colMap);
                await bulk.WriteToServerAsync(reader, cancellationToken);
            }

            private static IEnumerable<SqlBulkCopyColumnMapping> GetColumnMappings<T>() =>
                ColumnMapping.GetOrAdd(typeof(T),
                    type =>
                        type.GetProperties()
                            .Select(p => new SqlBulkCopyColumnMapping(p.Name, p.Name)).ToArray());
        }
    }

Updated version:

I also added excel extension change and moving excel into another folder. Any comments?

public static class Program
    {
        private static async Task ReadAllExcelFilesInDirectory(string path, List<User> usersList)
        {
            var dirInfo = new DirectoryInfo(path);
            var files = dirInfo.GetFiles("*.xlsx");
            foreach (var file in files)
                await ReadExcelFile(file, usersList);
        }

        private static async Task ReadExcelFile(FileInfo fileName, List<User> usersList)
        {
            const string destinationFile = @"T:\Test";

            if (fileName == null) throw new ArgumentNullException(nameof(fileName));
            if (usersList == null) throw new ArgumentNullException(nameof(usersList));

            using var package = new ExcelPackage(fileName);
            ExcelPackage.LicenseContext = LicenseContext.NonCommercial;
            var currentSheet = package.Workbook.Worksheets;
            var workSheet = currentSheet.First();
            var noOfCol = workSheet.Dimension.End.Column;
            var noOfRow = workSheet.Dimension.End.Row;
            for (var rowIterator = 2; rowIterator <= noOfRow; rowIterator++)
            {
                var user = new User
                {
                    GameCode = workSheet.Cells[rowIterator, 1].Value?.ToString(),
                    Count = Convert.ToInt32(workSheet.Cells[rowIterator, 2].Value),
                    Email = workSheet.Cells[rowIterator, 3].Value?.ToString(),
                    Status = Convert.ToInt32(workSheet.Cells[rowIterator, 4].Value)
                };


                usersList.Add(user);
            }

            //Change extension
            await using var newfile = File.OpenWrite(fileName.FullName + ".old");
            await package.SaveAsAsync(newfile);
            // To move a file or folder to a new location
            File.Move(fileName.FullName, destinationFile + "\\" + fileName.Name);
        }

        private static async Task InsertUsersIntoDatabase(List<User> usersList)
        {
            var conn = ConfigurationManager.ConnectionStrings["Development"].ConnectionString;
            await using var connString = new SqlConnection(conn);
            connString.Open();
            await InsertAsync(usersList, "[GameAPI].[dbo].[A101Orders]", connString, CancellationToken.None);
        }

        
        private static async Task Main(string[] args)
        {
            try
            {
                var usersList = new List<User>();
                await ReadAllExcelFilesInDirectory(
                    @"C:\Test", usersList);
                await InsertUsersIntoDatabase(usersList);

               
            }
            catch (Exception e)
            {
                Console.WriteLine(e);
                throw;
            }
        }

        private static async Task InsertAsync<T>(IEnumerable<T> items, string tableName, SqlConnection connection,
            CancellationToken cancellationToken)
        {
            using var bulk = new SqlBulkCopy(connection);
            await using var reader = ObjectReader.Create(items);
            bulk.DestinationTableName = tableName;
            var properties = typeof(T).GetProperties();
            foreach (var prop in properties)
                bulk.ColumnMappings.Add(new SqlBulkCopyColumnMapping(prop.Name, prop.Name));
            await bulk.WriteToServerAsync(reader, cancellationToken);
        }

        
    }
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Rather than extending the code in a question that has been answered, it might be better to ask a follow on question with a link to this question. \$\endgroup\$
    – pacmaninbw
    Nov 15, 2021 at 15:05

1 Answer 1

3
\$\begingroup\$

Missing classes

You have failed to include your definition of the User class in the code snippet so I had to create my own.

class User
{
    public string GameCode { get; set; }
    public int Count { get; set; }
    public string Email { get; set; }
    public int Status { get; set; }
}

For ConfigurationManager there are 3 different packages that I could install.

  • Microsoft.IdentityModel.Protocols
  • System.Configuration.ConfigurationManager
  • NLog

After consulting my magic 8-ball, I decided to go for System.Configuration.ConfigurationManager.

Incorrect usage of try...catch

Placing your entire program into one try...catch block serves no purpose and makes debugging much harder.

Splitting concerns into functions

Currently, your main program does it all. Consider splitting it out into the following functions:

class Program
{
    static void ReadExcelFile(string fileName, List<User> usersList)
    {
        // ...
    }

    static void ReadAllExcelFilesInDirectory(string path, List<User> usersList)
    {
        var dirInfo = new DirectoryInfo(path);
        var files = dirInfo.GetFiles("*.xlsx");
        foreach (var file in files)
            ReadExcelFile(file.FullName, usersList);
    }

    static async void InsertUsersIntoDatabase(List<User> usersList)
    {
        // ...
    }

    static async Task Main(string[] args)
    {
        var usersList = new List<User>();
        ReadAllExcelFilesInDirectory(@"C:\Test", usersList);
        InsertUsersIntoDatabase(usersList);
    }
}

Opening the database connection

Here you have mixed up conn and connString.

var conn = ConfigurationManager.ConnectionStrings["Development"].ConnectionString;
await using var connString = new SqlConnection(conn);
connString.Open();

The following would have made much more sense:

var connString = ConfigurationManager.ConnectionStrings["Development"].ConnectionString;
using var conn = new SqlConnection(connString);
await conn.OpenAsync();

BulkWriter

I think you are overcomplicating things here. The following is much simpler and easier to read:

class BulkWriter
{
    public static async Task InsertAsync<T>(IEnumerable<T> items, string tableName, SqlConnection connection,
        CancellationToken cancellationToken)
    {
        using var bulk = new SqlBulkCopy(connection);
        await using var reader = ObjectReader.Create(items);
        bulk.DestinationTableName = tableName;
        var properties = typeof(T).GetProperties();
        foreach (var prop in properties)
            bulk.ColumnMappings.Add(new SqlBulkCopyColumnMapping(prop.Name, prop.Name));
        await bulk.WriteToServerAsync(reader, cancellationToken);
    }
}

You can actually remove the whole BulkWriter class and simply make InsertAsync a method of Program

Unused variables

Right at the top on Main, you have this line that does not do anything:

string s = null;

Final thoughts

Every time you make an asynchronous call, you immediately use await to block execution until it is done. Is it really worth it?

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your thoughts, I will try splitting concerns into functions and bulkwriter simplification. \$\endgroup\$
    – raysefo
    Nov 11, 2021 at 16:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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