Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make FontData key in map immutable #1994

Merged
merged 1 commit into from
Apr 10, 2025

Conversation

ShahzaibIbrahim
Copy link
Contributor

Creating clone of FontData to be used as a key in map of ScalingSWTFontRegistry and DefaultSWTFontRegistry to make it immutable and inaccessible from outside of the class

Copy link
Contributor

github-actions bot commented Apr 4, 2025

Test Results

   539 files  ±0     539 suites  ±0   35m 0s ⏱️ + 4m 54s
 4 332 tests ±0   4 322 ✅ ±0    9 💤 ±0  1 ❌ ±0 
16 587 runs  ±0  16 483 ✅ ±0  103 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 493c646. ± Comparison against base commit c2136c9.

♻️ This comment has been updated with latest results.

@ShahzaibIbrahim ShahzaibIbrahim marked this pull request as draft April 4, 2025 11:17
@ShahzaibIbrahim ShahzaibIbrahim force-pushed the master-118 branch 2 times, most recently from d64b33a to 76cde38 Compare April 4, 2025 11:20
@ShahzaibIbrahim ShahzaibIbrahim marked this pull request as ready for review April 4, 2025 11:31
Creating clone of FontData to be used as a key in map of
ScalingSWTFontRegistry and DefaultSWTFontRegistry to make it immutable
and inaccessible from outside of the class
@fedejeanne
Copy link
Contributor

Failing test is unrelated #1843

@fedejeanne fedejeanne merged commit 4466c96 into eclipse-platform:master Apr 10, 2025
13 of 17 checks passed
@@ -72,7 +72,8 @@ public Font getFont(FontData fontData, int zoom) {
}

private Font registerFont(FontData fontData, Font font) {
fontsMap.put(fontData, font);
FontData clonedFontData = new FontData(fontData.toString());
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a heavy-weight operation, creating a String just to parse it again.
Isn't there a (internal) copy-constructor or copy method that handles this more efficiently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make FontData key in map of ScalingSWTFontRegistry immutable
3 participants