refactor: P3 code quality improvements across translation services#72
refactor: P3 code quality improvements across translation services#72
Conversation
- DoubaoService: replace List<string> + string.Join with StringBuilder for consistency with GeminiService and BaseOpenAIService; rename RemoveSurroundingQuotes to CleanupResult to match naming convention - GeminiService: remove unused StringBuilder buffer in ParseGeminiStreamAsync - HotkeyService: add volatile modifier to _isDisposed and _isInitialized for defensive consistency with MouseHookService - DeepLService: merge duplicate GetDeepLLanguageCode/GetDeepLWebLanguageCode into single method with isWeb parameter (only difference: Portuguese PT vs PT-PT), removing ~28 lines of duplication https://claude.ai/code/session_01Sz7TonncZGm4PgXYBzPEBo
There was a problem hiding this comment.
Pull request overview
Refactors several translation services and a WinUI hotkey service to reduce duplication, remove unused code, and align implementation patterns across providers.
Changes:
- DoubaoService: accumulate streaming chunks via
StringBuilderand rename the post-processing helper toCleanupResult. - DeepLService: consolidate API/web language-code mapping into a single method with an
isWebparameter. - GeminiService / HotkeyService: remove an unused variable and mark state flags as
volatile.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dotnet/src/Easydict.WinUI/Services/HotkeyService.cs | Marks initialization/disposal flags as volatile. |
| dotnet/src/Easydict.TranslationService/Services/GeminiService.cs | Removes an unused StringBuilder in stream parsing. |
| dotnet/src/Easydict.TranslationService/Services/DoubaoService.cs | Switches to StringBuilder for chunk aggregation and refactors/renames result cleanup logic. |
| dotnet/src/Easydict.TranslationService/Services/DeepLService.cs | Removes duplicated language-code mapping by unifying web/API code paths with a parameterized helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Clean up translated text by trimming and removing surrounding quotes. | ||
| /// </summary> | ||
| private static string RemoveSurroundingQuotes(string text) | ||
| private static string CleanupResult(string text) | ||
| { | ||
| if (string.IsNullOrEmpty(text)) | ||
| return text; |
There was a problem hiding this comment.
The new CleanupResult helper claims to "trim" in its XML summary, but its current implementation returns the original untrimmed text when no surrounding quotes are found (so TranslateInternalAsync no longer reliably trims output as it did before). Adjust CleanupResult so the trimming behavior is consistently applied.
| var sb = new StringBuilder(); | ||
| await foreach (var chunk in TranslateStreamAsync(request, cancellationToken)) | ||
| { | ||
| chunks.Add(chunk); | ||
| sb.Append(chunk); | ||
| } | ||
|
|
||
| var translatedText = string.Join("", chunks).Trim(); | ||
| translatedText = RemoveSurroundingQuotes(translatedText); | ||
| var translatedText = CleanupResult(sb.ToString()); | ||
|
|
||
| return new TranslationResult | ||
| { |
There was a problem hiding this comment.
Consider adding a unit test that covers whitespace-only differences (e.g., a streamed result with trailing newlines/spaces and no surrounding quotes) to ensure CleanupResult/TranslateAsync preserves the intended trimming behavior going forward.
Code ReviewI've reviewed this PR and found 1 bug that needs to be addressed: Bug:
|
2 participants
https://claude.ai/code/session_01Sz7TonncZGm4PgXYBzPEBo