This is a first post about great code review feedback, that I either gave or received. It will always consist of 3 parts: context, review feedback and explanation. You can go ahead and read previous ones here: https://www.michalbialecki.com/2019/06/21/code-reviews/. So lets not wait anymore and get to it.

The context

This is a simple ASP.Net application, that is requesting database to get count of elements filtered by one parameter. In this case we need a number of users providing a country code, that is always two character string.

This is how DB schema looks like:

Code in .net app is written with Dapper nuget package, that extends functionalities of IDbCommand and offers entities mapping with considerably good performance. It looks like this:

public async Task<IEnumerable<UserDto>> GetCountByCountryCode(string countryCode)
{
    using (var connection = new SqlConnection(ConnectionString))
    {
        return await connection.QueryAsync<UserDto>(
            "SELECT count(*) FROM [Users] WHERE CountryCode = @CountryCode",
            new { CountryCode = countryCode }).ConfigureAwait(false);
    }
}

Looks pretty standard, right? What is wrong here then?

Review feedback

Please convert countryCode parameter to ANSI string in GetCountByCountryCode method, cause if you use it like that, it’s not optimal.

Explanation

Notice, that CountryCode in database schema is a varchar(2) and this means that it stores two 1-byte characters. On the contrary nvarchar type is 2-byte per character type, that can store multilingual data. When using .net String type we are using unicode strings by default and therefore if we pass countryCode string to SQL it will have to be converted to ANSI string first.

The correct code should look like this:

public async Task<IEnumerable<UserDto>> GetCountByCountryCodeAsAnsi(string countryCode)
{
    using (var connection = new SqlConnection(ConnectionString))
    {
        return await connection.QueryAsync<UserDto>(
            "SELECT count(*) FROM [Users] WHERE CountryCode = @CountryCode",
            new { CountryCode = new DbString() { Value = countryCode, IsAnsi = true, Length = 2 } })
            .ConfigureAwait(false);
    }
}

If we run SQL Server Profiler and check what requests are we doing, this is what we will get:

As you can see first query needs to convert CountryCode parameter from nvarchar(4000) to varchar(2) in order to compare it.

In order to check how that would impact the performance, I created a SQL table with 1000000(one milion) records and compared results.

Before review it took 242 miliseconds and after review it took only 55 miliseconds. So as you see it is more that 4 times performance improvement in this specific case.

  All code posted here you can find on my GitHub: https://github.com/mikuam/console-app-net-core