Promote renderResult method to public #1801
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I think that's OK. My request would be to fill out javadoc for this method then, and if you have a sec, any basic unit test for the method too |
cdcarloschacon
force-pushed
the
MakeRenderResultMethodPublic
branch
2 times, most recently
from
c4aa812
to
27e3059
Compare
|
@srowen , the javadoc was added to the target method and some junit tests were added too. By the way, I moved the "renderResult" method to the begin of the class, taking into consideration that it is now a |
srowen
requested changes
May 1, 2024
core/src/test/java/com/google/zxing/qrcode/QRCodeWriterTestCase.java
Outdated
Show resolved
Hide resolved
| @@ -37,6 +37,60 @@ public final class QRCodeWriter implements Writer { | |||
|
|
|||
| private static final int QUIET_ZONE_SIZE = 4; | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
I'm not against moving this, but it'd be slightly cleaner to not move the method
|
I will fix that ASAP ... thanks for the insights ...
…On Wed, May 1, 2024, 9:16 AM Sean Owen ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In core/src/test/java/com/google/zxing/qrcode/QRCodeWriterTestCase.java
<#1801 (comment)>:
> @@ -133,4 +136,55 @@ public void testRegressionTest() throws Exception {
"renderer-test-01.png");
}
+ @test
+ public void renderResultScalesNothing() {
+ final int expected_size = 33; // Original Size (25) + quietZone
Oh, this fails style checks. Just expectedSize
------------------------------
In core/src/main/java/com/google/zxing/qrcode/QRCodeWriter.java
<#1801 (comment)>:
> @@ -37,6 +37,60 @@ public final class QRCodeWriter implements Writer {
private static final int QUIET_ZONE_SIZE = 4;
+ /**
I'm not against moving this, but it'd be slightly cleaner to not move the
method
—
Reply to this email directly, view it on GitHub
<#1801 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABPGBRUUV2PQGUUKVEFIT53ZAD2NBAVCNFSM6AAAAABG7SJOYOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZTG4YDOOJWGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This small commit makes public the "renderResult" method of the
"QRCodeWriter" class.
The reason to promote this method as public is that I'm facing
Business Case where, besides of generating the QR code as a PNG image,
I also need to print information about the QR code itself (specifically,
the QR version). Navigating through the code of zxing library, I noticed
that the com.google.zxing.qrcode.encoder.Encoder allows me to have that
(the QR metadata); however, by invoking that class I only have part of
the equation (the QRCode DTO) ... I'm still needing to generate the
BitMatrix in order to produce the QR image.
Here, I have two alternatives: either I invoke the Encoder.encode
class by myself and repeat the same logic when invoking the
QRCodeWriter.encode method; or I make the renderResult Public, call the
Encoder.encode and then I proceed to call the QRCodeWriter.renderResult
method with the previous result.
cdcarloschacon
force-pushed
the
MakeRenderResultMethodPublic
branch
from
27e3059
to
8e20b8c
Compare
|
The style issues were fixed ... also, the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This small commit makes public the "
renderResult" method of the "QRCodeWriter" class.The reason to promote this method as public is that I'm facing business case where, besides of generating the QR code as a PNG image, I also need to print information about the QR code itself (specifically, the QR version). Navigating through the code of zxing library, I noticed that the
com.google.zxing.qrcode.encoder.Encoderallows me to have that (the QR metadata); however, by invoking that class I only have part of the equation (theQRCodeDTO) ... I'm still needing to generate theBitMatrixin order to produce the QR image.Here, I have two alternatives: either I invoke the
Encoder.encodeclass by myself and repeat the same logic when invoking theQRCodeWriter.encodemethod; or I make therenderResultPublic, call theEncoder.encodeand then I proceed to call theQRCodeWriter.renderResultmethod with the previous result.