-
Notifications
You must be signed in to change notification settings - Fork 185
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
Add Base64 missing tests and documentation fixes #161
Add Base64 missing tests and documentation fixes #161
Conversation
public void testEncodeToString() { | ||
fail("Not yet implemented"); | ||
final Base64 base64 = new Base64(); | ||
final byte[] bytesToEncode = new byte[] {'l', 'i', 'g', 'h', 't', ' ', 'w', 'o', 'r'}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random question here; was the example intended to be "light work" encoded? 🙂 Asking as it was used in the other tests, but doesn't really matter as this test is working fine 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, just a random example from Base64 - Output Padding page in wikipedia 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Random question here; was the example intended to be "light work" encoded? 🙂 Asking as it was used in the other tests, but doesn't really matter as this test is working fine 👍
In the future, we could split up test method per test source, like testWikipediaExamplesAbc
, testRfc123FeatureX
, and so on.
* | ||
* @param pArray A byte array containing Base64 character data | ||
* @return a byte array containing binary data | ||
* @return a byte array containing binary data; will return {@code null} if provided byte array is null. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/is null/is {@code null}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 7c11889
@@ -114,10 +114,10 @@ public class Base64 { | |||
// some state be preserved between calls of encode() and decode(). | |||
|
|||
/** | |||
* Tests a given byte array to see if it contains only valid characters within the Base64 alphabet. | |||
* Tests a given byte array to see if it contains any valid character within the Base64 alphabet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I had to read the code to confirm it. To me the updated text looks correct, great catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Commits need to be squashed but we can take care of that later. Thanks @jkbkupczyk !
FYI @garydgregory @kinow