I think a lot about how to write maintainable code. It’s not all about services.
Let’s say you have this bit of code:
var maxRetries = 3;
var attempts = 0;
var success = false;
var lastError = “”;
do
{
try
{
ConnectToEmitter();
}
catch( Exception ex )
{
attempts++;
lastError = ex.Message;
Trace.WriteLine( $”failed: {ex.Message}” );
}
} while( attempts<maxRetries & !success );
if( !success )
{
SendWarning( $”failed: {lastError}” );
}This is from a real project I’m working on for a message driven dashboard/info-radiator. I may write more about that in the future but for now let’s discuss how you communicate the intent of the code above.
This code it’s pretty clear that what it’s doing is trying “ConnectToEmitter().” If it fails, it tries again until it either connects or exceeds maxRetries. Upon success/failure some related actions should happen.
This is very common code. You can use something like Polly (https://github.com/App-vNext/Polly). This project is being written for Android devices using Xamarin.Forms. Not every framework works on the various platforms, such as Xamarin.Forms. I find that on Xamarin I have to roll-my-own more often than I’d like.
Jeff is a master at the fluid interface. I implemented the above code as a fluid interface and it came out like this:
Attempt
.ThisAction( ConnectToEmitter )
.MaxNumberOfAttempts( 20 )
.DelayBetweenAttempts( TimeSpan.FromSeconds( 2 ) )
.Succeed( () => ConMan.OnConnected?.Invoke() )
.Fail( ( pErr ) => SendWarning( $”failed: {pErr}” ) )
.FailedAttempt( ( pErr ) => Trace.WriteLine( $”failed: {pErr}” ) )
.Go();My question to you is, “Is one solution better than the other? Is one more clear? Is one more maintainable?”
It’s not quite a slam dunk for the fluid interface. First, it’s way more code. The 20 lines in the original block became about 100 lines in the class “Attempt.” That’s 5 times the amount of code. “Attempt” is reusable and the original block is a one off. I have many places to use this code. It’s also more flexible than the original block, but is it “better?” Moreover, how do you explain it?
Here I’ve replaced the parameters with a made up syntax explaining each call. The format is <Type:descriptive-text>.
Attempt
.ThisAction( <Action:action-delegate> )
.MaxNumberOfAttempts( <int:number-of-attempts> )
.DelayBetweenAttempts( <TimeSpan:timespan-to-delay-between-each-attempt> )
.Succeed( <Action:action-delegate-executes-on-success-of-ThisAction> )
.Fail( <Action<string>:action-delegate-executes-on-fail-of-ThisAction-passing-the-last-error-message> )
.FailedAttempt( <Action<string>:action-delegate-executes-on-fail-of-each-attempt-of-ThisAction-passing-the-last-error-message> )
.Go(); // Begins the attempts to execute “ThisAction”What I don’t like about the original block is the number of places you can break it. If you change the tracking variables it breaks. If you change the internal blocks, it breaks. If you change the try/catch block, it breaks. The success/failure code is dependent on a variable that falls out (success).
The problem is not that the original block is so hard to understand (it’s only 20 lines of code, after all). The maintainability problem is that it’s so easy to break. Any refactoring stands a high chance of causing it to fail.
The “Attempt” class codifies the intent of the original. The single class completely controls usage. It is hard to break because the consumer of the code cannot change it’s behavior. It’s worth noting that services share this feature. It’s also why services should generally implement “core” code. You don’t want them to change. In our case we try to limit common changes to the managers.
What I’ve done with the “Attempt” class is to make “Attempting an Action multiple times” a core function. I’ve encapsulated the behavior. If you need different behavior it’s unlikely you’ll break this code. You’ll have to do a completely different implementation.
The dashboard I’m working has inspired several these type of classes. My process has been: 1) get it working, 2) try to make my intent clear, 3) try to extract the essential algorithm to a framework class.
It’s been interesting. This is an message driven system. The implications of that are that you must deal with error conditions which you cannot fix. You must detect the error condition and then wait, periodically trying to “reconnect.” This is much harder than it sounds. You do not control the flow of messages. Sometimes you need a certain message. For example, upon reconnecting after a failure how do you get the latest info?
In a pure messaged based system you would publish a message to do everything. Except transmitting control messages is tricky. What if only one of several devices was having a problem? If you send a “I’m reconnected” message will it causes the other displays to reset? What if they are currently displaying valid messages?
I know what you’re thinking. You can embed device ids into the control messages. That’s fine, but now the consumers of those messages have to know about those ids. Every system needs to know the ids and what to do with them. Do you want to complicate the system that way? Maybe…
Meanwhile, your code needs to be clean, it’s intent clear. And so I think about maintainable code.
BTW, there’s a very interesting slight of hand going on in this code. It’s a slight-of-hand that developers don’t know they do. It’s the cause of many problems and goes almost undetected by your average developer. I’m pointing it out to make it explicit how I’ve tricked you while you’ve read these messages, which I know you have.
The slight-of-hand is that I *taught* you how to use the fluid code by teaching you (showing you) the original.
I’ve suggested that the code in the class “Attempt” is better. I’ve suggested that it’s clearer and easier to maintain and understand.
Those things are true only because I *taught* you how to use it with the original. Had I only shown you the fluid code would you understood it? For example, how would you know that “MaxAttempts” is the number of times to try only if all the attempts fail? How would you know that it doesn’t mean to try “at least *MaxAttempts*?” It’s because I showed you the original code.
This point is lost on 99% of developers. They explain “Attempt” not showing the original and everybody gets lost.
Developers as a rule don’t understand their own mind games. Those of us that code for a living need to understand these points to communicate well. 99% of developers would create Attempt, then expect their peers to use it. *Aint gunna happen.* Now you know why.
