Implemented overloaded readFileToByteArray() method #457
Implemented overloaded readFileToByteArray() method #457jmloudis wants to merge 2 commits intoapache:masterfrom
Conversation
…[] based on offset and length parameters
Codecov Report
@@ Coverage Diff @@
## master #457 +/- ##
============================================
- Coverage 84.47% 84.43% -0.04%
- Complexity 3291 3294 +3
============================================
Files 223 223
Lines 7935 7954 +19
Branches 949 953 +4
============================================
+ Hits 6703 6716 +13
- Misses 981 983 +2
- Partials 251 255 +4
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
garydgregory
left a comment
garydgregory
left a comment
There was a problem hiding this comment.
-1: Does not match the ticket but the ticket is closed anyway.
| final byte[] arr = new byte[] {10, 20, 30, 40 ,50, 60, 70}; | ||
| Files.write(file.toPath(), arr); | ||
|
|
||
| final byte[] data = FileUtils.readFileToByteArray(file, 0, 0); |
There was a problem hiding this comment.
The resulting arrays are off by 1 in length, if I ask to read a length of 0, i should get back a byte array of length 0.
| * Reads the contents of a file into a byte[]. Start reading from offSet in file index to specific length. | ||
| * | ||
| * @param file the file to read, nust not be {@code null} | ||
| * @param arrayOffset read from an offset in the file |
There was a problem hiding this comment.
Parameters are misnamed: This is the index in the source file.
| * | ||
| * @param file the file to read, nust not be {@code null} | ||
| * @param arrayOffset read from an offset in the file | ||
| * @param arrayLength read file up to certain length based on index |
There was a problem hiding this comment.
This is how much to read from the file.
| if (offset < 0 ) { | ||
| throw new IllegalArgumentException("Offset must be grater than or equal to zero: " + offset); | ||
| } | ||
| if (length < offset) { |
There was a problem hiding this comment.
The index and length are unrelated, the length is the length of the desired result. The question is what should happen if there is not enough file data.
|
Thank you for the feedback! I will look fixing these issues this weekend. |
| Files.write(file.toPath(), arr); | ||
|
|
||
| final byte[] data = FileUtils.readFileToByteArray(file, 0, 0); | ||
| assertEquals(0, data.length);// return one byte |
There was a problem hiding this comment.
The comment is wrong: How is length zero equal to one byte?
| Files.write(file.toPath(), arr); | ||
| FileUtils.readFileToByteArray(file, 2, 1); | ||
| } catch (IOException | IllegalArgumentException ex) { | ||
| assertEquals("Length must be greater than or equal to offset: " + 2, ex.getMessage()); |
There was a problem hiding this comment.
This makes no sense to me: the position and length should not be related; "2, 1" should mean, starting at 0-based position 2, read one byte. IOW, I am describing the same semantics as 'java.io.InputStream.read(byte[], int, int)'. In this test, the file is 7 bytes in length, so "2, 1", should return the third byte value of "30".
|
Closing, no reply in 3 weeks. |
3 participants
Hello team,
Implemented a new overloaded readFileToByteArray() method that creates byte[] based on offset and length parameters. This is a new feature that was request by the following jira ticket.