<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hello Alan and everyone else<br>
    <br>
    Thank you for review!<br>
    <br>
    Here's another version of webrev:<br>
    <a href="http://cr.openjdk.java.net/%7Eigerasim/8020669/5/webrev/">http://cr.openjdk.java.net/~igerasim/8020669/5/webrev/</a><br>
    <br>
    The comments are inline.<br>
    <br>
    <div class="moz-cite-prefix">On 26.07.2013 8:54, Alan Bateman wrote:<br>
    </div>
    <blockquote cite="mid:51F200F1.8010500@oracle.com" type="cite">
      <meta content="text/html; charset=ISO-8859-1"
        http-equiv="Content-Type">
      On 25/07/2013 16:56, Ivan Gerasimov wrote:
      <blockquote cite="mid:51F1BB22.6040405@oracle.com" type="cite"><br>
        Would you please help review an updated webrev? <br>
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Eigerasim/8020669/4/webrev/">http://cr.openjdk.java.net/~igerasim/8020669/4/webrev/</a>
        <br>
      </blockquote>
      Sorry for the late response to your mails on this, I was on
      vacation.<br>
      <br>
      As you have found, in the original implementation we allowed for
      resizing (truncation or extending) while the file was open but
      when it was optimized to avoid an unnecessary resize of the array
      then we lost the ability to deal with extending case (which is the
      essentially the same as when the size is reported is less than the
      actual size).<br>
      <br>
      Reverting to the original implementation and doing a single read
      to double check for EOF seems reasonable. <br>
      <br>
      My only real comment on the latest webrev (4) is that the
      computation of the new capacity is not obvious to the reader (and
      since this is the uncommon case then it doesn't need to be
      super-optimized).&nbsp; So I think it would be a lot clearer if
      "newCapacity" were re-introduced and then computed to a value that
      is a minimum of capacity+2, maximum of capacity+BUFFER_SIZE (with
      OOME handling when newCapacity is greater than MAX_BUFFER_SIZE). I
      think that would be clearer than re-using the remaining count
      (rem).<br>
    </blockquote>
    <br>
    OK. I simplified it once again.<br>
    Now the new capacity is set to one of {BUFFER_SIZE, 2 * old
    capacity, MAX_BUFFER_SIZE}, whatever is appropriate.<br>
    I think that doubling the capacity is better than increasing it by
    BUFFER_SIZE, since this will allow to allocate needed buffer in less
    number of iterations.<br>
    <br>
    <blockquote cite="mid:51F200F1.8010500@oracle.com" type="cite"> <br>
      A minor comment is that the input stream is named "fis" but
      probably should be "in" as it is not a FileInputStream (I see
      David Llyod suggested using a FileInputStream but the path may be
      to a file in a custom file system so it can't be a
      FileInputStream).<br>
      <br>
    </blockquote>
    Renamed.<br>
    <br>
    <blockquote cite="mid:51F200F1.8010500@oracle.com" type="cite"> I
      see you've changed the javadoc from "Read" to "Reads" on several
      methods. I don't know if you meant to include this in the webrev
      but personally prefer "Read".<br>
      <br>
    </blockquote>
    Files.java has mixed style. Most methods are documented with "Does"
    form, but some have "Do".<br>
    I left "Reads" on the methods I modify for consistency, but reverted
    changes from other places as I'm not going to change all the
    occurrences in this webrev.<br>
    <br>
    <blockquote cite="mid:51F200F1.8010500@oracle.com" type="cite">
      Thanks for moving the test to BytesAndLines as that is the test
      for these methods. One suggestion is to move the new test for
      readAllBytes into
      <meta http-equiv="content-type" content="text/html;
        charset=ISO-8859-1">
      testReadAndWriteBytes and the new test for <span class="new">readAllLines

        into testReadLines. It doesn't matter of course, it just keeps
        them together.<br>
        <br>
      </span></blockquote>
    Sure, done.<br>
    <br>
    Sincerely yours,<br>
    Ivan<br>
    <br>
    <blockquote cite="mid:51F200F1.8010500@oracle.com" type="cite"><span
        class="new"> -Alan.<br>
      </span> </blockquote>
    <br>
  </body>
</html>