This function started very clean but I had to add requirements:
Added a
Stopwatch
to only update the caller periodically. I guess that responsibility could be handled elsewhere but I'm undecided about whether or not it should be.Added a
Stopwatch
to only check a computationally expensive operation periodically.Give the user the option to cancel the whole thing.
Additionally, I'd like for the enclosing class to be static so that calls to it look like this:
var encoded = Codec.Encode(myImage, myMessage, myCheckCancelMethod);
var decoded = Codec.Decode(myImage, myCheckCancelMethod);
There is a corresponding Encode
method here that should match as closely as possible. The code works (UWP and Android so far, you have to use a .png right now).
What I see as being the problem is this function is supposed to just call out to other functions but there is so much coordination going on that the coordination function has gotten complicated and you can't really take anything out.
public static async Task<byte[]> Decode(Bitmap carrierImage, byte[] eofMarker, Func<double, bool> checkCancel)
{
var shuffleSeed = FisherYates.GetSeed(eofMarker);
bool foundEof = false;
bool userCancelled = false;
List<bool> messageBuilder = new List<bool>();
byte[] decodedMessage = null;
var userUpdateStopwatch = new Stopwatch();
var eofStopwatch = new Stopwatch();
userUpdateStopwatch.Start();
eofStopwatch.Start();
await Task.Run(() => // move away from the calling thread while working
{
IterateBitmap(carrierImage, shuffleSeed, (x, y) => {
var pixelBitsAsBools = DecodePixel(carrierImage, x, y);
messageBuilder.AddRange(pixelBitsAsBools);
if(eofStopwatch.ElapsedTicks > EOF_CHECK_RATE)
{
eofStopwatch.Reset();
foundEof = MessageBuilderHasEof(messageBuilder, eofMarker);
}
var percentComplete = (double)messageBuilder.Count / carrierImage.BitCapacity;
userCancelled = CheckCancelAndUpdate(userUpdateStopwatch, percentComplete, checkCancel);
return userCancelled || foundEof;
});
foundEof = MessageBuilderHasEof(messageBuilder, eofMarker); // check for eof again in case the IterateBitmap loop completed since the last time we checked.
if (foundEof)
{
decodedMessage = GetMessageWithoutEof(messageBuilder, eofMarker);
}
});
return userCancelled || !foundEof ? null : decodedMessage;
}
Func
; 2) Why don't you use theCancellationToken
for cancelling? \$\endgroup\$ – t3chb0t Jun 19 '18 at 7:55