Do it only when you need it

Writing good quality code is somehow jumbled and not an easy task to do. To be honest, it's a matter of your experience - the more code you write, the easier is to make it reusable and easy to understand. As a beginner I tended to write many layers of abstraction, which were supposed to separate all the concerns. At least it's something many books and tutorials for "rookies" say - create a lot of abstraction, use this, use that - it will help making your code maintainable. To make a long story short - it's a complete bullshit.

The more abstraction you introduce in your code, the harder it is to understand it. You can divide your 50 LOC method into three different ones, which JIT compiler will inline anyway - but is it something you're doing because you know it's the right thing to do? You can introduce all those builder, singleton, abstract factory patterns - but are there any evidences you're delivering any value to the codebase? In most cases we're doing something because a mysterious man - the author of the most popular C++/C#/Java book - said so. We're building the foundations of our skills basing on fairy tales, which are completely irrelevant in a real world.

Very few people proclaim a very easy and straightforward rule - do it when you need it.

The rules are simple. Don't introduce a pattern if don't need it yet(and never introduce it, if you don't understand it). Don't introduce an abstraction, if you're not sure what your code is going to look like. Don't divide your methods or classes just for a sake of dividing them. Whatever refactoring you're about to do - do it only if you need it.

I believe this is from all the misunderstanding of TDD comes from - people have learned, that they should refactor their code always, no matter how it looks like. They forget, that they should use their intuition rather than blindly following all the rules.

Ldstr, where are you?

In the meantime I'm working on a Fody add-in, which can validate SQL query if it finds one. Details will be provided soon - now I'd like to describe one issue I had when analyzing MSIL code for it

Internally this add-in scans MSIL instructions in an assembly trying to find ldstr opcode in methods' bodies. There is no magic - I haven't found better way to find strings in the generated code, mostly because strings are not decorated in MSIL in any way. It worked just fine for most of test cases I created. I decided to test it in my side-project against more realistic use cases. The output turned out to be somehow surprising:

Fody: Fody (version 1.29.4.0) Executing
Fody/Stamp:   Starting search for git repository in SolutionDir
Fody/Stamp:   Found git repository in E:\Codenova\Klienci\MikroSystem\vNext\.git\
Fody/QueryValidator:   Found 0 queries to validate.
Fody/QueryValidator:   Trying to get configuration for E:\Codenova\Klienci\MikroSystem\vNext\LicznikNET.vNext.Api\obj\Debug\LicznikNET.vNext.Api.exe
Fody/QueryValidator:   Found configuration file E:\Codenova\Klienci\MikroSystem\vNext\LicznikNET.vNext.Api\app.config
Fody/QueryValidator:   Connection string is Data Source=.\SQLEXPRESS;Initial Catalog=vNext;Persist Security Info=True;User ID=foo;Password=bar;MultipleActiveResultSets=True;

What the heck?! I have almost 100 SQL queries in my code and still it found nothing? Impossibru!

I decided to run dotPeek and do some "visual debugging". Let's say we have following method:

public void Bar()
{
     using (var connection = new SqlConnection())
     {
          connection.Query(@"|> SELECT * FROM dbo.Foo");
     }
}

This works perfectly fine, the generated MSIL is what I expected:

IL_0006: ldloc.0      // connection
IL_0007: ldstr        "|> SELECT * FROM dbo.Foo"
IL_000c: ldnull       
IL_000d: ldnull       
IL_000e: ldc.i4.1     
IL_000f: ldloca.s     V_1
IL_0011: initobj      valuetype [mscorlib]System.Nullable`1<int32>
IL_0017: ldloc.1      // V_1
IL_0018: ldloca.s     V_2
IL_001a: initobj      valuetype [mscorlib]System.Nullable`1<valuetype [System.Data]System.Data.CommandType>
IL_0020: ldloc.2      // V_2
IL_0021: call         class [mscorlib]System.Collections.Generic.IEnumerable`1<object> [Dapper]Dapper.SqlMapper::Query(class [System.Data]System.Data.IDbConnection, string, object, class [System.Data]System.Data.IDbTransaction, bool, valuetype [mscorlib]System.Nullable`1<int32>, valuetype [mscorlib]System.Nullable`1<valuetype [System.Data]System.Data.CommandType>)
IL_0026: pop    

But this doesn't reproduce my problem because in my side-project all my DB calls are asynchronous(using Dapper of course). Fixed example could look like this:

public async Task Foo()
{
     using (var connection = new SqlConnection())
     {
          await connection.QueryAsync(@"|> SELECT * FROM dbo.Foo");
     }
}

Now, when I check MSIL, the root of all evil will be revealed:

IL_0000: ldloca.s     V_0
IL_0002: call         valuetype [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder::Create()
IL_0007: stfld        valuetype [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder QueryValidator.Fody.TestWeb.TestClass/'<Foo>d__0'::'<>t__builder'
IL_000c: ldloca.s     V_0
IL_000e: ldc.i4.m1    
IL_000f: stfld        int32 QueryValidator.Fody.TestWeb.TestClass/'<Foo>d__0'::'<>1__state'
IL_0014: ldloc.0      // V_0
IL_0015: ldfld        valuetype [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder QueryValidator.Fody.TestWeb.TestClass/'<Foo>d__0'::'<>t__builder'
IL_001a: stloc.1      // V_1
IL_001b: ldloca.s     V_1
IL_001d: ldloca.s     V_0
IL_001f: call         instance void [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder::Start<valuetype QueryValidator.Fody.TestWeb.TestClass/'<Foo>d__0'>(!!0/*valuetype QueryValidator.Fody.TestWeb.TestClass/'<Foo>d__0'*/&)
IL_0024: ldloca.s     V_0
IL_0026: ldflda       valuetype [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder QueryValidator.Fody.TestWeb.TestClass/'<Foo>d__0'::'<>t__builder'
IL_002b: call         instance class [mscorlib]System.Threading.Tasks.Task [mscorlib]System.Runtime.CompilerServices.AsyncTaskMethodBuilder::get_Task()
IL_0030: ret  

When implementing my add-in I forgot, that compiler converts all async calls into state machines(or to be more specific - into nested classes implementing IAsyncStateMachine interface). Because I wasn't scanning nested classes(I believe it was a bug anyway), no async query could be found. After loading instructions for nested classes also, I am sure it works as intended:

Fody: Fody (version 1.29.4.0) Executing
Fody/Stamp:   Starting search for git repository in SolutionDir
Fody/Stamp:   Found git repository in E:\Codenova\Klienci\MikroSystem\vNext\.git\
Fody/QueryValidator:   Found 94 queries to validate.
Fody/QueryValidator:   Trying to get configuration for E:\Codenova\Klienci\MikroSystem\vNext\LicznikNET.vNext.Api\obj\Debug\LicznikNET.vNext.Api.exe
Fody/QueryValidator:   Found configuration file E:\Codenova\Klienci\MikroSystem\vNext\LicznikNET.vNext.Api\app.config
Fody/QueryValidator:   Connection string is Data Source=.\SQLEXPRESS;Initial Catalog=vNext;Persist Security Info=True;User ID=foo;Password=bar;MultipleActiveResultSets=True;