Now that we have implemented some benchmarks in #6 we can use them to investigate some feedback I saw in an assisted code review.
Here is the method in question from lib/src/decoder.dart
// Base32Decoder class
@override
Uint8List convert(String input) {
var bits = 0;
var value = 0;
// The output buffer.
final output = <int>[];
for (var i = 0; i < input.length; i++) {
var char = input[i];
int? charValue;
switch (_variant) {
case Base32Variant.rfc4648:
case Base32Variant.rfc4648Hex:
// Skip padding characters.
if (char == '=') continue;
charValue = _variant.getCharacterIndex(char);
case Base32Variant.crockford:
char = char.toUpperCase();
if (char == 'O') char = '0';
if (char == 'I' || char == 'L') char = '1';
// Crockford alphabet does not have padding.
charValue = _variant.getCharacterIndex(char);
}
// Converts the characters to their 5-bit binary values.
value = (value << 5) | charValue;
bits += 5;
// Converts the 5-bit values to the corresponding 8-bit values.
// These values are then stored in the output array.
if (bits >= 8) {
output.add((value >> (bits - 8)) & 255);
bits -= 8;
}
}
return Uint8List.fromList(output);
}Issue: The synchronous convert method builds a List output and then returns Uint8List.fromList(output). This involves growing a list and then performing a full copy into a new Uint8List.
Recommendation: For a high-performance codec, it's better to pre-allocate the Uint8List to its exact final size and write directly into it. The decoded length can be calculated from the input string length (ignoring padding).
Suggested Refactor
@override
Uint8List convert(String input) {
var bits = 0;
var value = 0;
// Remove padding for length calculation
final relevantInput = input.endsWith('=') ? input.replaceAll('=', '') : input;
final decodedLength = (relevantInput.length * 5) ~/ 8;
// The output buffer.
final output = Uint8List(decodedLength);
var outputIndex = 0;
for (var i = 0; i < relevantInput.length; i++) {
var char = relevantInput[i];
int? charValue;
switch (_variant) {
case Base32Variant.rfc4648:
case Base32Variant.rfc4648Hex:
// Padding is already removed, so no need to check
charValue = _variant.getCharacterIndex(char);
case Base32Variant.crockford:
char = char.toUpperCase();
if (char == 'O') char = '0';
if (char == 'I' || char == 'L') char = '1';
charValue = _variant.getCharacterIndex(char);
}
// Converts the characters to their 5-bit binary values.
value = (value << 5) | charValue;
bits += 5;
// Converts the 5-bit values to the corresponding 8-bit values.
// These values are then stored in the output array.
if (bits >= 8) {
output[outputIndex++] = (value >> (bits - 8)) & 255;
bits -= 8;
}
}
return output;
}
Now that we have implemented some benchmarks in #6 we can use them to investigate some feedback I saw in an assisted code review.
Here is the method in question from
lib/src/decoder.dartIssue: The synchronous convert method builds a List output and then returns Uint8List.fromList(output). This involves growing a list and then performing a full copy into a new Uint8List.
Recommendation: For a high-performance codec, it's better to pre-allocate the Uint8List to its exact final size and write directly into it. The decoded length can be calculated from the input string length (ignoring padding).
Suggested Refactor