Thursday, April 4, 2019

Tidying up a project - safe refactoring rules

With 6 kids (!) that I love with every fiber of my being, I'm used to rooms being a bit of a mess when I enter. Therefore, I try to lead by example and:
"Make the space better for me having been there"
Which means, depending on where I am, I wipe down the sink in the bathroom, put a dish or two in the dishwasher, etc. Of course, any child within earshot, when appropriate, gets a "everything on pause, come help me with this..."

And, like all rules, there is nuance. When we're out the door for a thing in 2 minutes, I don't do this. Everybody needs to stay on task. I'm not going to distract from the shoe tying & coat buttoning to get "the dang toys off of Bruce (our table, don't ask) and into a bin!"

Likewise, when I open up a C# programming project - .sln in Visual Studio - to do a thing, I will accompany that thing with some refactorings that feel generally safe.

Nuance: I calculate the days to when this code will hit my production environment (the bake time). If we're below my comfort zone, I do not do any of these steps. I don't have a hard cutoff for comfort zone, and much of it depends on the project. When we're talking hours to prod and not days or weeks, my comfort zone is a little on edge, so I say skip these. From one of my favorite writings
"vi. Break any of these rules sooner than say anything outright barbarous." - George Orwell
Ok, enough with the boring. Onto my rules
NOTE: I'm nowhere near as good of a C# developer as I am C++. These rules are going to change for me over time. Also, the point of this is to not proscribe a set of rules for the reader, but to get you thinking in terms of what are YOUR safe refactoring rules.

My Safe Refactoring Rules


Definitions

  • TEST projects are C# projects (.csproj) that contain the unit tests, integration tests, smoke tests, whatever tests. The build artifacts produced do NOT run on production. The build artifacts produced by a TEST project are used to exercise the artifacts that DO run on production. 
  • PRODUCTION projects are the C# projects that produce build artifacts that DO run on production.

Rules

  1. Validate that all TEST projects are on the latest department accepted version of the .NET Framework
  2. Make sure that all projects are configured so that StyleCop style violations are errors, not warnings.
  3. Update all NuGet packages in TEST projects, except for the department documented "don't update" packages.
  4. For each .cs file that is modified, make sure that unnecessary using statements are removed.
    • Visual Studio has a function under Edit / IntelliSense / Organize Usings / Remove Unnecessary Usings. I have muscle memory around alt e i o r.
  5. For each .cs file that is modified, sort your using statements.
    • Visual Studio has a function under Edit / IntelliSense / Organize Usings / Sort Usings. I have muscle memory around alt e i o s.
  6. If your PRODUCTION projects are developing NuGet packages, avoid updating dependent NuGet packages unless absolutely necessary.
    • Final choice of the proper version consumed should be up to the application, not the NuGet package.
    • NuGet package dependencies here are an expression of "what interface do I expect."

Deets

I want to go into a bit of detail about each of the choices.

1. Update .NET Framework

Developers should be familiar with how to update the target .NET Framework and not allow code to languish on an old framework version, like some kind of an animal. Tools like the Target Framework Migrator can help tremendously. 

2. StyleCop

While I can be annoyed at StyleCop, it's an "eat your broccoli" to me, as I think projects are generally better with StyleCop than without. 

I like StyleCop.Error.MSBuild to turn StyleCop warnings into errors, although it needs a hand modification to your packages.config file to turn it into a development dependency

<package id="StyleCop.MSBuild" version="5.0.0" targetFramework="net461" developmentDependency="true" />
lest a build choice on a package be injected into a downstream consumer that is not quite ready for it.

Sometimes this choice can cause me to have to fix more warnings-turned-errors than I'm comfortable with, so it may turn into scheduled work for the future.

3. Update all NuGet packages in TEST projects

Developers should be "watching the skies" for updates to NuGet packages, as stale packages can cause bugs (been there, done that). 

By doing this in a safe manner, where the only thing you may break are your tests (which would reject your build because you gate, right, RIGHT?) gets you in a good mindset.

If your department does not have a documented list of "don't update these" packages, then demand one. Better yet, create & champion one.

4. & 5. Fix yer usings

Using statements are happier when they are organized this way; I've spoken to them.

6. Be careful with package to package dependencies

While this feels like a weird one, as it's not a "do this" rule, but an "avoid this", I thought it was important to call out the difference between what a package dependency represents when developing a NuGet package and what a package dependency represents when developing an application.

While developing a NuGet package, you're selecting an expected interface by the version choices of your package dependencies.

If you find bugs with particular releases of your dependencies, then by all means, express that in your dependency ("Anybody that uses package FOOBAR version 3.1.5 or below is a FOOL and will get the BAZ race condition bug - Minimum expected version is 3.1.6").

Other than bugs, you should let the application be the final arbiter of what version of package they are dependent upon. When wearing the NuGet package developer hat, you can't determine the full context in which your package will be used, and the application may be combining your package with other packages written by other developers in interesting ways that require more control over dependencies.

Being as flexible as possible over dependent versions helps here.

Conclusion

Again, these are my rules, not yours. My rules will change over time as I get more seasoned in C#. The purpose of this is to get you thinking about what your "tidying up" rules might be so your projects stay fresh and moving forward.

No comments:

Post a Comment