Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-03-01 - [Fast String Concatenation and Secure Random]

**Learning:** String concatenation using `+=` inside loops is very slow for large iterations (O(N^2)). Using `StringBuffer` or allocating a `List<int>` and using `String.fromCharCodes` is significantly faster (O(N)). Additionally, `Random.secure()` has a slight performance overhead but is essential for IDs to prevent predictability. Caching `Random.secure()` instances avoids initialization overhead.
**Action:** Replace `+=` string concatenation with `String.fromCharCodes` inside random string generators. Cache `Random.secure()` to save instantiation time.
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@ class AutoIdGenerator {
static const String alphabet =
'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';

static final Random _random = Random();
static final Random _secureRandom = Random.secure();
static final _alphabetCodeUnits = alphabet.codeUnits;

/// Automatically Generates a random new Id
static String autoId() {
final stringBuffer = StringBuffer();
const maxRandom = alphabet.length;

for (var i = 0; i < idLength; ++i) {
stringBuffer.write(alphabet[_random.nextInt(maxRandom)]);
}

return stringBuffer.toString();
final codes = List<int>.generate(
idLength,
(_) =>
_alphabetCodeUnits[_secureRandom.nextInt(_alphabetCodeUnits.length)],
);
return String.fromCharCodes(codes);
}
}
19 changes: 10 additions & 9 deletions lib/common_domain_models/lib/src/ids/src/id.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

import 'dart:math';

final _secureRandom = Random.secure();
const _chars = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This character set is also defined in lib/abgabe/abgabe_client_lib/lib/src/models/auto_id_generator.dart and lib/sharezone_utils/lib/src/random_string/random_string.dart. To improve maintainability and prevent inconsistencies, it would be best to define this alphabet as a single shared constant. If package dependencies prevent sharing, please ensure the string values are identical across all files. Note that the character order in this file is different from the others.

final _charCodes = _chars.codeUnits;

class Id {
final String value;

Expand All @@ -21,15 +25,12 @@ class Id {
/// Generates a new random [Id] with the given [length] using characters
/// from a-z, A-Z and 0-9.
static Id generate({int length = 20, Random? random}) {
random ??= Random();
const chars =
'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789';
final id =
List.generate(
length,
(index) => chars[random!.nextInt(chars.length)],
).join();
return Id(id);
final rand = random ?? _secureRandom;
final codes = List<int>.generate(
length,
(_) => _charCodes[rand.nextInt(_charCodes.length)],
);
return Id(String.fromCharCodes(codes));
}

@override
Expand Down
21 changes: 11 additions & 10 deletions lib/sharezone_utils/lib/src/random_string/random_string.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,23 @@

import 'dart:math';

final _secureRandom = Random.secure();
const _idChars =
'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
final _idCharCodes = _idChars.codeUnits;

String randomString(int length) {
var rand = Random();
var codeUnits = List.generate(length, (index) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The local variable codeUnits is never reassigned, so it should be declared as final to promote immutability. This aligns with the repository style guide's recommendation to follow a consistent rule for var and final (line 101) and matches the use of final for similar variables elsewhere in this pull request.

Suggested change
var codeUnits = List.generate(length, (index) {
final codeUnits = List.generate(length, (index) {
References
  1. The style guide recommends following a consistent rule for using var and final for local variables. Since this variable is not reassigned, using final is more appropriate and consistent with other changes in this PR. (link)

return rand.nextInt(33) + 89;
return _secureRandom.nextInt(33) + 89;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The magic numbers 33 and 89 obscure the purpose of this code. To improve readability, please replace them with named constants that explain what they represent (e.g., the range and starting character code for the random string generation). For example:

const _charRange = 33;
const _charStartCode = 89; // ASCII for 'Y'
...
return _secureRandom.nextInt(_charRange) + _charStartCode;

});

return String.fromCharCodes(codeUnits);
}

String randomIDString(int length) {
var rand = Random();
const chars =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
String result = "";
for (var i = 0; i < length; i++) {
result += chars[rand.nextInt(chars.length)];
}
return result;
final codes = List<int>.generate(
length,
(_) => _idCharCodes[_secureRandom.nextInt(_idCharCodes.length)],
);
return String.fromCharCodes(codes);
}
Loading