fix: resolve SonarCloud issues in remaining renderers#85
Conversation
Co-Authored-By: Afonso Dutra Nogueira Filho <afonsoft@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
3efb71b
into
feature/devin-20260712-sonar-quality
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| public class Base64QRCode : AbstractQRCode | ||
| { | ||
| private QRCode qr; | ||
| private readonly QRCode qr; |
There was a problem hiding this comment.
📝 Info: Making qr readonly cements a pre-existing inconsistency in SetQRCodeData
The Base64QRCode.SetQRCodeData override at QRCoder.Core/Renderers/Base64QRCode.cs:90-93 delegates to this.qr.SetQRCodeData(data) but never calls base.SetQRCodeData(data). This means the parent class's QrCodeData property is never updated when using this method. This is a pre-existing issue (not introduced by this PR), but adding readonly to the qr field makes it slightly harder to refactor in the future — e.g., you can't reassign qr to a new QRCode(data) instance. In practice this doesn't matter because Base64QRCode only uses qr (not QrCodeData directly) for rendering, but it's worth being aware of the asymmetry if the class is ever extended.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (!string.IsNullOrEmpty(colorString) && colorString[0] == '#') | ||
| colorString = colorString.Substring(1); |
There was a problem hiding this comment.
📝 Info: HexSKColorToByteArray condition refactor is behaviorally equivalent
The condition was changed from colorString.StartsWith("#") to !string.IsNullOrEmpty(colorString) && colorString[0] == '#'. These are equivalent for all practical inputs: for null, both paths crash (old crashes at StartsWith, new crashes at colorString.Length on the next line); for empty string, both skip the if and return an empty byte array; for #-prefixed strings, both strip the #. The new form avoids a string allocation from StartsWith and adds a null guard that doesn't fully protect (since the next line still dereferences colorString). If full null safety is desired, an early ArgumentNullException would be clearer.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private string RenderGraphic(Base64QRCodeGraphicOptions options, ImageType imgType) | ||
| { | ||
| var base64 = string.Empty; | ||
| using (SKBitmap bmp = qr.GetGraphic(new QRCodeGraphicOptions | ||
| { | ||
| PixelsPerModule = options.PixelsPerModule, | ||
| DarkSKColor = options.DarkSKColor, | ||
| LightSKColor = options.LightSKColor, | ||
| DrawQuietZones = options.DrawQuietZones, | ||
| Icon = options.Icon, | ||
| IconSizePercent = options.IconSizePercent, | ||
| IconBorderWidth = options.IconBorderWidth | ||
| })) | ||
| { | ||
| base64 = SKBitmapToBase64(bmp, imgType); | ||
| } | ||
| return base64; | ||
| } |
There was a problem hiding this comment.
📝 Info: RenderGraphic method was only relocated, not modified
The RenderGraphic method was moved from before GetGraphic(int, SKColor, SKColor, ...) to after it. The code is identical (only whitespace/indentation of the object initializer changed). The diff makes it look like a deletion and re-addition, but no logic was changed.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
All Submissions:
New Feature Submissions:
Changes to Core Features:
fix: resolve SonarCloud issues in remaining renderers
This PR addresses the remaining SonarCloud code-smell issues in the renderers that were not covered in Phase 3:
SvgQRCode.cs: reduced method visibility by making helper methodsstatic, marked fields inSvgLogoasreadonly, removed redundant: intenum underlying type, suppressedS3776on the SVGGetGraphicmethod andS107on the helper.Base64QRCode.cs: movedRenderGraphicso allGetGraphicoverloads are adjacent, madeqrreadonly andSKBitmapToBase64static, suppressedS107/S1133on the obsoleteGetGraphicoverload andS107on the helper.AsciiQRCode.cs/PngByteQRCode.cs/PdfByteQRCode.cs/PostscriptQRCode.cs: madeHexSKColorToByteArray/CleanSvgValstatic, replacedStartsWith("#")withstring[0] == '#'fornet48compatibility, and suppressedS107onGetQRCodeconvenience helpers.PdfByteQRCode.GetGraphicalso suppressesS1192for the unavoidable repeated PDF literal fragments.Build and
dotnet test QRCoder.Core.Tests/ -f net10.0 --configuration Releasepass locally (502 tests).Link to Devin session: https://app.devin.ai/sessions/20c92a1bad724f63b8be89a9e195e664
Requested by: @afonsoft