Jeremy Davis
Jeremy Davis
Sitecore, C# and web development
Article printed from: https://blog.jermdavis.dev/posts/2023/inheritance-bug-static-methods

How to waste a day on an inheritance bug

Proving to myself how much I have forgotten over the years

Published 13 February 2023
.Net C# ~3 min. read

I've been migrating a big chunk of .Net 4 code to .Net 7 recently. One of the few large changes I had to make was to replace some boilerplate generation that used T4 Templates with a Source Generator. (As T4 isn't entirely supported in latest .Net) But these work very differently, so that change involved a good chunk of work. But I messed this up in a way that caused a subtle bug. And while I may well get to writing about Source Generators later, that silly bug is also worth writing up. Even if it's just to remind me not to make the same mistake in the future...

The issue

The code I was working on was part of an event system. For each of the possible events there's an object which describes one of two possible messages around that event: A "can this event occur?" message which asks the entities in the system if they will allow the event, and then a "this event has occurred!" message which is sent if (when) the event does occur. Given there were a lot of possible events in the system, and 95% of the code in these classes followed some simple patterns, partial classes were being generated from data to save on typing boilerplate.

Making a simple example, a generated event object for DoThing might look like:

public class DoThing
{
	private string _thing;
	
	public string Thing => _thing;
	
	protected DoThing(string thing)
	{
		_thing = thing;
	}

	public static DoThing Create(string thing)
	{
		return new DoThing($"DoThing: {thing}");
	}
	
	public static DoThing Create(int id)
	{
		return new DoThing($"DoThing: {id}");
	}
}

					

It's got some data to record the thing being done (and the entities involved), plus some helper methods to correctly construct the event for a couple of scenarios. (In the original code that was one for "make it from live objects" and one for "make it from the IDs of objects which aren't currenly in memory")

And then the generated CanDoThing event object was pretty much the same except for its name and its helper methods:

public class CanDoThing : DoThing
{
	protected CanDoThing(string thing) : base(thing) {}
	
	public static new CanDoThing Create(int id)
	{
		return new CanDoThing($"CanDoThing: {id}");
	}
}

					

And then in the logic of the app, it would create the approprate objects and use them vaguely like:

var canWalk = CanDoThing.Create("Walk");
if(CurrentEntities.Query(canWalk))
{
   var walking = DoThing.Create("Walk");
   CurrentEntities.Notify(walking);
}


					

And that sort of pattern was used quite a lot around the codebase. Each entity would have some logic in it that made appropriate tests based on "Can" events it received and if it was being asked to do something it did not want to allow it would return false, to stop the event.

But after the migration to Source Generators this was broken. Everything compiled fine, but its behaviour was wrong when executed. The Query() calls were always passing, even in situations where they should have prevented the event. And I'll admit I wasted a long time staring at this code and failing to see the issue...

What I broke

My "aha!" moment came when I added some debuugging code to the app. I picked a place where some events were being queried and added some code to show what was happening. For the basic example above, it was effectively:

var canWalk = CanDoThing.Create("Walk");
Console.WriteLine(canWalk.Thing);

					

Based on the hacky example above, the output of that should be CanDoThing: Walk. The code is clearly calling a helper-constructor on CanDoThing so that is the object type I was expecting to be returned. But in reality, the output of the code was DoThing: Walk.

That explained the code failing - the entities were all looking for Can<something> events but they were getting <something> events instead. So their "will I allow this event?" logic never saw the query events it was supposed to, and hence allowed everything.

But why was that happening? Clearly the code is calling CanDoThing. How can it get back a different type?

Why it's broken

Those of you who's brains are more switched on than mine was at the time may well already have spotted the issue here. Inheritence. It works even with static helper methods...

If you look back at my simple example above, the DoThing type defines a Create(string) helper method and a Create(int) one too. But the CanDoThing example inherits from DoThing but only provides a replacement for the Create(int) method. That means when the code said CanDoThing.Create("Walk") the compiler was actualling calling DoThing.Create("Walk") because that's the only method with the right signature.

When I'd migrated the T4 Template to the a Source Generator I'd accidentally missed the second helper method from the code that generates the Can<something> classes, and everywhere the code was trying to create a Can<something> event it was actually createing a <something> event.

The fix was easy - adding the logic to ensure the Can<something> objects look like this:

public class CanDoThing : DoThing
{
	protected CanDoThing(string thing) : base(thing) {}
	
	public static new CanDoThing Create(int id)
	{
		return new CanDoThing($"CanDoThing: {id}");
	}

	public static new CanDoThing Create(string thing)
	{
		return new CanDoThing($"CanDoThing: {thing}");
	}
}

					

With that added, everything went back to working. And I got to mull over how I'd managed to forget how objects work when I'd started looking at this bug.

Plus I had an idea for some extra unit tests, to catch any repeat of this sort of mistake in the future...

↑ Back to top