These days I had to rename all the projects inside a Visual Studio solution and the folders containing them. From SomeThing.Core to Something.Core. That wasn’t the exact typo. But that’s the idea. Here it’s what I learned. It wasn’t as easy as only renaming the projects in Visual Studio.
1. Rename folders and projects manually
After digging into it, I found this StackOverflow answer to rename folders and projects inside a solution. It layouts a checklist of what to do. Here I’m bringing it with some extra steps:
First, close Visual Studio.
Then, create a backup of the .sln file. Just in case.
Next, use git mv to rename the folder from SomeThing to Something. This preserves the history of the file. Also, rename the csproj files to match the new name.
Use the next bash script to rename the folders and csproj files inside the src and tests folders.
# Replace these two values, pleaseold=SomeThing
new=Something
# Put here all others folders...for folder in'Core''Infrastructure'do
git mv src/$old.$folder src/$old.$folder.Temp
git mv src/$old.$folder.Temp src/$new.$folder
git mv src/$new.$folder/$old.$folder.csproj src/$new.$folder/$new.$folder.csproj
git mv tests/$old.$folder.Tests tests/$old.$folder.Tests.Temp
git mv tests/$old.$folder.Tests.Temp tests/$new.$folder.Tests
git mv src/$new.$folder/$old.$folder.csproj src/$new.$folder/$new.$folder.csproj
done
Notice that it uses a temp folder to rename the containing SomeThing folder. I found that workaround in this StackOverflow answer. Git doesn’t go along with different casings in file names.
Next, in the .sln file, edit all instances of SomeThing to be Something, using a text editor like NotePad++.
Next, restart Visual Studio, and everything will work as before, but with the project in a different directory.
Next, right-click on the solution name and run “Sync Namespaces.”
Fix all namespaces with 'Sync Namespaces' option
Next, use “Replace in Files” to clean leftovers. For example, launchSettings.json and appSettings.json files.
Fix all other leftovers with 'Replace In Files' option
Everything should compile. Horraaay!
That was a tedious process. Especially if, like me, you have to rename all source and tests projects in a big solution.
Given the amount of votes of the StackOverflow answer I followed, more than 400, I bet somebody else thought about automating this process.
Indeed.
2. Rename folders and projects with ProjectRenamer
ProjectRenamer is a dotnet tool that does all the heavy and repetitive work for us. It renames the folders, csproj files, and project references. Also, it could create a Git commit and build the solution.
ProjectRenamer expects the Git working directory to be clean. Stash or commit all other changes before using it.
Using a Powershell prompt, from the folder containing the .sln file, run:
With the --no-commit flag, ProjectRenamer will only stage the files. And, the --no-review skips the user confirmation. It’s like the -f flag of some Unix commands.
Last time, in the Unit Testing 101 series, we refactored a unit test for a method that fed a report of transactions in a payment system. This time, let’s refactor another test. This test is based on a real test I had to refactor in one of my client’s projects.
Before looking at our test, a bit of background. This test belongs to a two-way integration between a Property Management System and a third-party service. Let’s call it: Acme Corporation. To connect one of our properties to Acme, we go throught an OAuth flow.
A bit of background on OAuth flows
To start the OAuth flow, we call an Authorize endpoint in a web browser. Acme prompts us to enter a user and password. Then, they return a verification code. With it, we call a Token endpoint to grab the authentication and refresh tokens. We use the authentication token in a header in future requests.
Apart from the authentication and refresh codes, to make this integration work in both ways, we create some random credentials and send them to Acme. With these credentials, Acme calls some public endpoints on our side.
Here’s the test to refactor
With this background, let’s look at the test we’re going to refactor. This is an integration test that checks that we can create, update and retrieve Acme “connections” in our database.
Yes, that’s the real test. “Some names have been changed to protect the innocent.” Can you take a look and identify what our test does?
To be fair, here’s the AcmeConnection class with the signature of Load() and other methods,
publicrecordLightspeedConnection(PmsPropertyIdPmsPropertyId){publicstaticAcmeConnectionLoad(AcmeConnectionIddbId,ClientIdclientId,AcmeCompany?acmeCompany=null,Pkce?pkce=null,AcmeCredentials?acmeCredentials=null,OurCredentials?ourCredentials=null){// Create a new AcmeConnection from all the parameters// Beep, beep, boop...}// A bunch of methods to update the AcmeConnection statepublicvoidGeneratePkce(){/* ... */}publicvoidUpdateAcmeCompany(AcmeCompanycompany){/* ... */}publicvoidUpdateAcmeCredentials(AcmeCredentialscredentials){/* ... */}publicvoidSetOurCredentialsAsync(IAcmeServiceservice){/* ... */}}
The Pkce object corresponds to two security codes we exchange in the OAuth flow. For more details, see Dropbox guide on PKCE.
Did you spot what our test does? Don’t worry. It took me some time to get what this test does, even though I was familiar with that codebase.
That test is full of noise and hard to follow. It abuses the acmeConnection variable. It keeps reading and assigning connections to it.
Behind all that noise, our test creates a new connection and stores it. Then, it retrieves, mutates, and updates the same connection. And in the last step, it recreates another one from all the input values to use it in the Assert part.
Let’s see the test again, annotated this time,
[Fact]publicasyncTaskGetConnectionAsync_ConnectionUpdated_ReturnsUpdatedConnection(){varrepository=newAcmeConnectionRepository(AnySqlConnection);varacmeConnection=newAcmeConnection(ClientId);varacmeConnectionId=awaitrepository.CreateAcmeConnectionAsync(acmeConnection);// 1. Create connection ^^^^^acmeConnection.GeneratePkce();acmeConnection=AcmeConnection.Load(acmeConnectionId,ClientId,pkce:acmeConnection.Pkce,acmeCredentials:AcmeCredentials,ourCredentials:OurCredentials.GenerateCredentials(ClientId));// ^^^^^// 2. Change both credentialsawaitrepository.UpdateAcmeConnectionAsync(acmeConnection);varconnectionFromDb=awaitrepository.GetAcmeConnectionAsync(ClientId);// ^^^^^// 3. Retrieve the newly created connectionacmeConnection=AcmeConnection.Load(acmeConnectionId,ClientId,AcmeCompany,connectionFromDb!.Pkce);// ^^^^^acmeConnection.GeneratePkce();// ^^^^acmeConnection=AcmeConnection.Load(acmeConnectionId,ClientId,AcmeCompany,acmeConnection.Pkce,connectionFromDb.AcmeCredentials,connectionFromDb.OurCredentials);acmeConnection.UpdateAcmeCredentials(OtherAcmeCredentials);// ^^^^^awaitacmeConnection.SetOurCredentialsAsync(_acmeConnectionServiceMock.Object);// ^^^^^// 4. Change Acme company and both credentials againawaitrepository.UpdateAcmeConnectionAsync(acmeConnection);// ^^^^^// 5. UpdatevarupdatedConnectionFromDb=awaitrepository.GetAcmeConnectionAsync(newClientId(ClientId));acmeConnection=AcmeConnection.Load(// ^^^^^acmeConnectionId,ClientId,AcmeCompany,Pkce.Load(acmeConnection.Pkce!.Id!,acmeConnection.Pkce.CodeVerifier,updatedConnectionFromDb.Pkce!.CreatedDate,updatedConnectionFromDb.Pkce.UpdatedDate),AcmeCredentials.Load(acmeConnection.AcmeCredentials!.Id!,acmeConnection.AcmeCredentials.RefreshToken,acmeConnection.AcmeCredentials.AccessToken,acmeConnection.AcmeCredentials.AccessTokenExpiration,updatedConnectionFromDb.AcmeCredentials!.CreatedDate,updatedConnectionFromDb.AcmeCredentials.UpdatedDate),OurCredentials.Load(acmeConnection.OurCredentials!.Id!,acmeConnection.OurCredentials.Username,acmeConnection.OurCredentials.Password,updatedConnectionFromDb.OurCredentials!.CreatedDate,updatedConnectionFromDb.OurCredentials.UpdatedDate));Assert.NotNull(connectionFromDb);Assert.NotNull(updatedConnectionFromDb);Assert.Equal(acmeConnectionId,connectionFromDb!.Id);Assert.Equal(acmeConnectionId,updatedConnectionFromDb!.Id);Assert.Equal(acmeConnection,updatedConnectionFromDb);Assert.NotEqual(acmeConnection,connectionFromDb);}
Also, this test keeps using the Load() method, even though the AcmeConnection class has some methods to update its own state.
Step 1. Use the same code as the Production code
Write integration tests using the same code as the production code.
Let’s write our test in terms of our business methods instead of using the Load() everywhere.
[Fact]publicasyncTaskGetConnectionAsync_ConnectionUpdated_ReturnsUpdatedConnection(){varrepository=newAcmeConnectionRepository(AnySqlConnection);varacmeConnection=newAcmeConnection(ClientId);varacmeConnectionId=awaitrepository.CreateAcmeConnectionAsync(acmeConnection);// 1. Create connection ^^^^^acmeConnection=awaitrepository.GetAcmeConnectionAsync(ClientId);acmeConnection.GeneratePkce();// ^^^^^awaitrepository.UpdateAcmeConnectionAsync(acmeConnection);// ^^^^^// 2. Update pkceacmeConnection=awaitrepository.GetAcmeConnectionAsync(ClientId);acmeConnection.UpdateAcmeCompany(AcmeCompany);// ^^^^^acmeConnection.UpdateAcmeCredentials(OtherAcmeCredentials);// ^^^^^awaitacmeConnection.SetOurCredentialsAsync(_acmeConnectionServiceMock.Object);// ^^^^^awaitrepository.UpdateAcmeConnectionAsync(acmeConnection);// ^^^^^// 3. Update company and credentialsvarupdatedConnectionFromDb=awaitrepository.GetAcmeConnectionAsync(ClientId);Assert.NotNull(updatedConnectionFromDb);Assert.Equal(acmeConnectionId,updatedConnectionFromDb!.Id);Assert.Equal(acmeConnection.Pkce,updatedConnectionFromDb.Pkce);Assert.Equal(acmeConnection.AcmeCompany,updatedConnectionFromDb.AcmeCompany);Assert.NotNull(updatedConnectionFromDb.AcmeCredentials);Assert.NotNull(updatedConnectionFromDb.OurCredentials);}
Notice, we stopped using the Load() method. We rewrote the test using the methods from the AcmeConnection class like UpdateAcmeCredentials, SetOurCredentialsAsync, and others.
Also, we separated the test into blocks. In each block, we retrieved the acmeConnection, mutated it with its own methods, and called UpdateAcmeConnectionAsync(). Cleaner!- I’d say.
We removed the last Load() call. We didn’t need to assert if the last retrieved object was exactly the same as the recreated version. Instead, we checked that the updated connection had the same value objects.
Step 2. Use descriptive variables
For the next step, let’s stop abusing the same acmeConnection variable and create more descriptive variables for every step.
With these variables names is easier to follow what our test does.
An alternative solution with Factory methods
We were lucky there were a lot of methods on the AcmeConnection class to mutate and update it in the tests. If we didn’t have those methods, we could create one “clone” method for every property we needed to mutate.
For example,
publicstaticclassAcmeConnectionExtensions{publicstaticAcmeConnectionCredentialsFrom(thisLightspeedConnectionself,AcmeCredentialsacmeCredentials,OurCredentialsourCredentials){// Copy self and change AcmeCredentials and OurCredentials}publicstaticAcmeConnectionAcmeCompanyFrom(thisLightspeedConnectionself,AcmeCompanyacmeCompany){// Copy self and change the AcmeCompany}}
We can create an initial AcmeConnection and clone it with our helper methods to reduce all boilerplate in our original test.
Voilà! That was a long refactoring session. There are two things we can take away from this refactoring. First, we should strive for readability in our tests. We should make our test even more readable than our production code. Can anyone spot what one of our tests does in 30 seconds? That’s a readable test. Second, we should always write our tests using the same code as our production code. We shouldn’t write production code to only use it inside our unit tests. That Load() method was a backdoor to build objects when we should have used class constructors and methods to mutate its state.
Names are important in programming. Good names could be the difference between a developer nodding his head in agreement or making funny faces in a “Wait, whaaaat?” moment. Names are so important that the Clean Code and The Art of Readable Code devote entire chapters to the subject. These are some words I’m banning from my method and class names.
1. Get and Set in method names
I wish I could remember what Kevlin Henney’s presentation has this idea. He argues that “Get” and “Set” are some words with more meanings in an English dictionary. Then why do we use them in our code when our names should be the least ambiguous as possible? He has a point!
These days I reviewed a pull request that had a code block that reminded me about this point. It looked like this,
Maybe WithReservationId() or simply ReservationId() would be better alternatives. Even an old auto-implemented property would get our backs covered here.
The next names I’m banning are the “Utility” and “Helper” suffixes in class names. Every time I see them, I wonder if the author (and I) missed an opportunity to create domain entities or better named classes.
In one of my client’s projects, I had to work with a class that looked like this,
publicstaticclassMetadataHelper{publicstaticvoidAddFeeRates(Feefee,PaymentRequestrequest,IDictionary<string,string>metadata){// Doing something with 'fee' and 'request' to populate 'metadata'...}publicstaticvoidAddFeeRates(Feefee,StripePaymentIntentpaymentIntent,IDictionary<string,string>metadata){// Doing something with 'fee' and 'paymentIntent' to populate 'metadata'...}}
It was a class that generated some payment metadata based on payment fees and requests. Somebody took the easy route and dumped everything in a static MetadataHelper class.
Instead, we could write a non-static PaymentMetadata class to wrap the metadata dictionary. Like this,
publicclassPaymentMetadata{privatereadonlyIDictionary<string,string>_metadata;publicPaymentMetadata(IDictionary<string,string>baseMetadata){_metadata=baseMetadata;}publicvoidAddFeeRates(Feefee,PaymentRequestrequest){// Doing something with 'fee' and 'request' to expand 'metadata'...}publicvoidAddFeeRates(Feefee,StripePaymentIntentpaymentIntent){// Doing something with 'fee' and 'paymentIntent' to expand 'metadata'...}publicIDictionary<string,string>ToDictionary()=>_metadata;}
If a concept is important inside the business domain, we should promote it out of helper classes.
Often, we use Utility and Helper classes to dump all kinds of methods we couldn’t find a good place for.
3. Constants classes
This isn’t exactly a name. But the last thing I’m banning is Constant classes. I learned this lesson after reading Domain Modeling Made Functional.
Recently, I found some code that looked like this,
It was a class full of unrelated constants. Here, I only showed five of them. Among those, I found the types of transactions in a reservation management system.
On the caller side, a method that expects any of the TransactionTypeId uses an int parameter. For example,
But, any int won’t work. Only those inside the Constants class are the valid ones.
This gets worse when Constant classes start to proliferate, and every project of a solution has its own Constants class. Arggggg!
Instead of Constants classes, let’s use enums to restrict the values we can pass to methods. Or, at least, let’s move the constants closer to where they’re expected, not in a catch-all class. With an enum, the compiler helps us to check if we are passing a “good” value.
Using an enum, our previous example looks like this,
Voilà! These are the names I’m banning in my own code. And I wish I could ban them in code reviews too. Are you also guilty of any of the three? I’ve been there and done that.
I like ASP.NET Core BackgroundServices. I’ve used them in one of my client’s projects to run recurring operations outside the main ASP.NET Core API site. Even for small one-time operations, I’ve run them in the same API site.
There’s one catch. We have to write our own retrying, multi-threading, and reporting mechanism. BackgroundServices are a lightweight alternative to run background tasks.
Lite Hangfire
These days, a coworker came up with the idea to use a “lite” Hangfire to replace ASP.NET Core BackgroundServices. By “lite,” he meant an in-memory, single-thread Hangfire configuration.
Let’s create an ASP.NET Core API site and install these NuGet packages:
To make things cleaner, let’s use extension methods to keep all Hangfire configurations in a single place. Like this,
usingHangfire;usingHangfire.Console;usingHangfire.InMemory;usingRecreatingFilterScenario.Jobs;namespaceLiteHangfire.Extensions;publicstaticclassServiceCollectionExtensions{publicstaticvoidConfigureHangfire(thisIServiceCollectionservices){services.AddHangfire(configuration=>{configuration.UseInMemoryStorage();// ^^^^^// Since we have good memoryconfiguration.UseConsole();// ^^^^^});services.AddHangfireServer(options=>{options.SchedulePollingInterval=TimeSpan.FromSeconds(5);// ^^^^^// For RecurringJobs: Delay between retries.// By default: 15secoptions.WorkerCount=1;// ^^^^^// Number of worker threads.// By default: min(processor count * 5, 20)});GlobalJobFilters.Filters.Add(newAutomaticRetryAttribute{Attempts=1// ^^^^^// Retry count.// By default: 10});}publicstaticvoidConfigureRecurringJobs(thisWebApplicationapp){//var config = app.Services.GetRequiredService<IOptions<MyRecurringJobOptions>>().Value;// ^^^^^^^^^// To read the cron expression from a config fileRecurringJob.AddOrUpdate<ProducerRecurringJob>(ProducerRecurringJob.JobId,x=>x.DoSomethingAsync(),"0/1 * * * *");// ^^^^^^^^^// Every minute. Change it to suit your own needsRecurringJob.Trigger(ProducerRecurringJob.JobId);}}
Notice that we used the UseInMemoryStorage() method to store jobs in memory instead of a database and the UseConsole() to bring color to our logging messages in the Dashboard.
In the previous step, when we registered the Hangfire server, we used these parameters:
SchedulePollingInterval is the time to wait between retries for recurring jobs. By default, it’s 15 seconds. Source
WorkerCount is the number of processing threads. By default, it’s the minimum between five times the processor count and 20. Source
As an aside, I also discovered these settings:
ServerCheckInterval is how often Hangfire checks for “timed out” servers. By default, it’s 5 minutes. Source
ServerTimeout is the time to consider that a server timed out from the last heartbeat. By default, it’s 5 minutes.
Then, we registered the number of retry attempts. By default, Hangfire retries jobs 10 times. Source
3. Write “Producer” and “Consumer” jobs
The next step is to register a recurring job as a “producer.” It looks like this,
usingHangfire;usingHangfire.Console;usingHangfire.Server;namespaceLiteHangfire.Jobs;publicclassProducerRecurringJob{publicconststringJobId=nameof(ProducerRecurringJob);privatereadonlyIBackgroundJobClient_backgroundJobClient;privatereadonlyILogger<ProducerRecurringJob>_logger;publicProducerRecurringJob(IBackgroundJobClientbackgroundJobClient,ILogger<ProducerRecurringJob>logger){_backgroundJobClient=backgroundJobClient;_logger=logger;}publicasyncTaskDoSomethingAsync(){_logger.LogInformation("Running recurring job at {now}",DateTime.UtcNow);// Beep, beep, boop...awaitTask.Delay(1_000);// We could read pending jobs from a database, for exampleforeach(variteminEnumerable.Range(0,5)){_backgroundJobClient.Enqueue<WorkerJob>(x=>x.DoSomeWorkAsync(null));// ^^^^^}}}
Inside this recurring job, we can read pending jobs from a database and enqueue a new worker job for every pending job available.
And a sample worker job that uses Hangfire.Console looks like this,
publicclassWorkerJob{publicasyncTaskDoSomeWorkAsync(PerformContextcontext){context.SetTextColor(ConsoleTextColor.Blue);context.WriteLine("Doing some work at {0}",DateTime.UtcNow);// Beep, beep, boop...awaitTask.Delay(3_000);}}
Notice that we expect a PerformContext as a parameter to change the color of the logging message. When we enqueued the worker jobs, we passed null as context, then Hangfire uses the right instance when running our jobs. Source.
Voilà! That’s how to use a lite Hangfire to replace BackgroundServices without adding too much overhead or a new database to store jobs. With the advantage that Hangfire has recurring jobs, retries, and a Dashboard out of the box.
“Ask questions” is common advice for better code reviews.
At some point, we followed that advice and started using what I call “leading” or “pushy” questions. Questions that only hint a request for a code change.
After working on a remote software team for a couple years, I stopped using “pushy” questions on code reviews. Here’s why it’s a bad idea.
“Pushy” Questions Are Time-Consuming
Let’s imagine we’ve written a method and forgot to check for nulls. Something like this,
If we follow the advice to ask “pushy” questions, we might leave and receive comments like “What if oneParam is null?” or “Could oneParam or anotherParam be null?”
The problem with those types of comments is we can’t tell if they’re genuine questions or actionable items. Is the reviewer asking a clarification question or “pushing” us in a different direction? We can’t tell.
Behind those comments, there’s a hidden change request. How is the code author supposed to know the reviewer is asking for a change?
While working on a remote team, it happened more than once that I had to reach out to reviewers via email or chat to ask them to clarify their intentions behind those comments. But some reviewers were in different time zones or even on the other side of the world. All interactions took about ~24 hours between my initial message and their response.
It was frustrating and time-consuming. Arrrggg!
When it was my turn to be a code reviewer, I chose a different approach: I stopped asking those questions.
Instead of asking “pushy” questions, let’s leave actionable and unambiguous comments that distinguish between questions, to-dos, and nice-to-haves.
Let’s go back to the previous example and leave an unambiguous comment. Like this one: “Is it possible that oneParam could be null? If that’s the case, please let’s add the appropriate null checks. Something like if (oneParam == null) throw ...“
With that comment, it’s clear we’re suggesting a change.
With that convention, we add keywords like “question,” “suggestion” or “nitpick” to clarify the purpose of our comments.
I used it for months in one of my client’s projects and other reviewers started to use it too.
For example, we can turn our previous “pushy” comment into these two depending on our intention:
A clarification question: “question: Is it possible that oneParam could be null?”
A change request: “suggestion (blocking): Let’s add the appropriate null checks if oneParam could be null.”
Now it’s clear we’re referring to two different actions.
Voilà! That’s why I don’t like “pushy” questions in code reviews. Let’s always prefer clear and direct comments without forgetting good manners of course. And let’s remember we review code from people with different experience levels and even non-native speakers of our language.
After this experience, my rule of thumb for better code reviews is to write unambiguous comments and always include a suggestion with each comment.
