<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFE0">
    Hi Joe,<br>
    <br>
    Looks good to me.<br>
    <br>
    Typo: StringCoding.java:1026 "unmappble"  (no new webrev needed)<br>
    <br>
    Regards, Roger<br>
    <br>
    <br>
    <div class="moz-cite-prefix">On 6/12/2018 12:52 PM, Joe Wang wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:5B1FFA39.9000704@oracle.com">Hi
      all,
      <br>
      <br>
      It's been a while since the 1st round of reviews. Thank you all
      for the valuable comments and suggestions!  Note that Roger's last
      response went to nio-dev, but not core-libs-dev, I've therefore
      attached it below.
      <br>
      <br>
      CSR [2]: the CSR is now approved. Note the write method has been
      changed to writeString.
      <br>
      <br>
      Impl [3]: for performance reason and the different requirement
      from byte-string conversion for malformed/unmappable character
      handing, the implementation uses a specific method separate from
      the existing ones. Both Sherman and Alan preferred specific method
      than adding parameters to the APIs that might be error prone.
      Thanks Sherman for the code!
      <br>
      <br>
      JMH benchmark tests showed the new read method performs better
      than an operation that reads the bytes and convert to string. The
      gains start to be significant even at quite reasonable sizes (<
      10K).
      <br>
      <br>
      <br>
      [1] JBS: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8201276">https://bugs.openjdk.java.net/browse/JDK-8201276</a>
      <br>
      [2] CSR: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8202055">https://bugs.openjdk.java.net/browse/JDK-8202055</a>
      <br>
      [3] webrevs:
      <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~joehw/jdk11/8201276/webrev/">http://cr.openjdk.java.net/~joehw/jdk11/8201276/webrev/</a>
      <br>
      <br>
      Please review.
      <br>
      <br>
      Thanks,
      <br>
      Joe
      <br>
      <br>
      On 5/15/18, 7:51 AM, Roger Riggs wrote:
      <br>
      <blockquote type="cite">Hi Joe, et. al.
        <br>
        <br>
        My $.02 on line separators:
        <br>
        <br>
         - We should avoid clever tricks trying to solve problems that
        are infrequent such as cross OS file line operations.
        <br>
            Most files will be read/written on a single system so the
        line separators will be as expected.
        <br>
        <br>
         - Have/use APIs that split lines consistently accepting both
        line separators so developer code can be agnostic to line
        separators.
        <br>
            aka BufferedReader.readLine for developers that are
        processing the contents *as lines*.
        <br>
           Those other methods already exist; if there are any gaps in
        line oriented processing that's a separate task.
        <br>
        <br>
         - These new file methods are defined to handle Charset
        encoding/decoding and buffering.
        <br>
            Since there are other methods to deal with files as lines
        these methods should not look for or break to lines.
        <br>
        <br>
         - Performance: adding code to look for line characters will
        slow it down and in most cases would have no impact
        <br>
            since the line endings will match the system.
        <br>
        <br>
         - The strings passed to writeString (CharSequence) should have
        been constructed using the proper line separators.
        <br>
            There are any number of methods that insert the os specific
        line terminator and its easy to write File.separator as needed.
        <br>
        <br>
        As for the method naming:
        <br>
        <br>
        I'd prefer "writeString" as a familiar term that isn't stretched
        too far by making the argument be a CharSequence.
        <br>
        its fine to call a CharSequence a string (with a lower case s).
        <br>
        <br>
        $.02, Roger
        <br>
        <br>
        <br>
        On 4/27/18 6:33 PM, Joe Wang wrote:
        <br>
        <blockquote type="cite">
          <br>
          <br>
          On 4/27/2018 12:50 PM, <a class="moz-txt-link-abbreviated" href="mailto:forax@univ-mlv.fr">forax@univ-mlv.fr</a> wrote:
          <br>
          <blockquote type="cite">----- Mail original -----
            <br>
            <blockquote type="cite">De: "Joe Wang"
              <a class="moz-txt-link-rfc2396E" href="mailto:huizhe.wang@oracle.com"><huizhe.wang@oracle.com></a>
              <br>
              À: "Remi Forax" <a class="moz-txt-link-rfc2396E" href="mailto:forax@univ-mlv.fr"><forax@univ-mlv.fr></a>, "Alan Bateman"
              <a class="moz-txt-link-rfc2396E" href="mailto:Alan.Bateman@oracle.com"><Alan.Bateman@oracle.com></a>
              <br>
              Cc: "nio-dev" <a class="moz-txt-link-rfc2396E" href="mailto:nio-dev@openjdk.java.net"><nio-dev@openjdk.java.net></a>,
              "core-libs-dev" <a class="moz-txt-link-rfc2396E" href="mailto:core-libs-dev@openjdk.java.net"><core-libs-dev@openjdk.java.net></a>
              <br>
              Envoyé: Vendredi 27 Avril 2018 21:21:01
              <br>
              Objet: Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to
              Files for reading/writing a string from/to a file
              <br>
              Hi Rémi, Alan,
              <br>
            </blockquote>
            Hi Joe,
            <br>
            <br>
            <blockquote type="cite">I'm not sure we'd want to replace
              system dependent line separators with
              <br>
              '\n'. There are cases as you described where the
              replacement makes
              <br>
              sense. However,  there are other cases too. For example,
              the purpose is
              <br>
              to read, edit a text file and then write it back. If this
              is on Windows,
              <br>
              and if line separators are replaced with '\n', that would
              cause the
              <br>
              resulting file not formatted properly in for example
              Notepad. There are
              <br>
              cases where users write text files on Windows using Java,
              and only found
              <br>
              the lines were not separated in Notepad.
              <br>
            </blockquote>
            I agree that why the counterpart of readString() that write
            the string should inserts the platform's line separator.
            <br>
            BTW, i just found that those methods are not named
            writeString, or writeCharSequence, i think they should.
            <br>
          </blockquote>
          <br>
          While readString() does not modify the original content (e.g.
          by replacing the platform's line separator with '\n'),
          write(String) won't either, by adding extra characters such as
          the line separator.
          <br>
          <br>
          I would think interfaces shall read along with the parameters.
          <br>
              readString(Path) == read as a String from the specified
          Path (one could argue for readToString, readAsString, but we
          generally omit the preps)
          <br>
              write(Path, CharSequence) == write the CharSequence to the
          file, since CharSequence is already in the method signature as
          a parameter, we probably don't want to add that to the name,
          otherwise it would read like repeating the word CharSequence.
          <br>
          <br>
          It is in a similar situation as write(Path, Iterable) where it
          was defined as writeLines(Path, Iterable).
          <br>
          <br>
          <blockquote type="cite">
            <br>
            <blockquote type="cite">Files.write(Path, Iterable) is also
              specified to terminate each line
              <br>
              with the platform's line separator. If readString does the
              replacement,
              <br>
              it would be inconsistent.
              <br>
            </blockquote>
            <br>
            Anyway, if we look for consistency the methods
            writeCharSequence should transform the \n in the
            CharSequence to the platform's line separator.
            <br>
            <br>
            Files.write(Path, Iterable) is is not a counterpart of
            readString(), it's consistent with Files.lines() or
            Files.readLines() (or BufferedReader.readLine()) that all
            suppress the line separators. Anyway, Files.write(path,
            readString(path)::line) will be consistent if you replace
            the line separators or not because String.line() suppresses
            the line separators.
            <br>
          </blockquote>
          <br>
          readString pairs with write(String), therefore it's more like
          Files.write(path, readString(path)) than
          readString(path)::line. The use case:
          <br>
          <br>
              String s = Files.read(path);
          <br>
              Files.write(path, s.replace("/config/location",
          "/new/location"));
          <br>
          <br>
          would then work as expected.
          <br>
          <br>
          These two methods are one-off (open/read/write/close file)
          operation. write(String) therefore is not intended for
          adding/appending multiple Strings from other operations such
          as String.line(). If an app needs to put the result of
          String.line() or any other processes into a file using this
          method, it needs to take care of adding the line separator
          itself when necessary. "when necessary" would be a judgement
          call by the developer.
          <br>
          <br>
          That said, if there's a consensus on the idea of terminating
          the string with a line separator, then readString method would
          need to strip it off to go with the write method.
          <br>
          <br>
          <blockquote type="cite">
            <br>
            <blockquote type="cite">I would think readString behaves
              like readAllBytes, that would simply read all content
              faithfully.
              <br>
            </blockquote>
            readString does an interpolation due to the Charset so it's
            not the real content, again, the idea is that developers
            should not have to care about the platform's line separators
            (they have more important things to think).
            <br>
            <br>
            The other solution is to just not introduce those new
            methods, after all
            Files.lines().collect(Collectors.joining("\n")) already does
            the job, no ?
            <br>
          </blockquote>
          <br>
          While there are many ways to do it, none is as
          straight-forward. "Read (entire) file to String"/"Write String
          to file" are popular requests from users. Read to string ->
          do some String manipulation -> write it back is such a
          simple use case, it really needs to be very easy to do as
          illustrated in the above code snippet.
          <br>
          <br>
          Cheers,
          <br>
          Joe
          <br>
          <br>
          <blockquote type="cite">
            <br>
            <blockquote type="cite">Best,
              <br>
              Joe
              <br>
            </blockquote>
            regards,
            <br>
            Rémi
            <br>
            <br>
            <blockquote type="cite">On 4/27/2018 4:43 AM,
              <a class="moz-txt-link-abbreviated" href="mailto:forax@univ-mlv.fr">forax@univ-mlv.fr</a> wrote:
              <br>
              <blockquote type="cite">Hi Alan,
                <br>
                <br>
                People do not read the documentation.
                <br>
                So adding something in the documentation about when a
                method should be used or
                <br>
                not is never a solution.
                <br>
                <br>
                Here the user want a String and provides a charset so
                you have no way but to
                <br>
                decode the content to substitute the line separator.
                <br>
                <br>
                cheers,
                <br>
                Rémi
                <br>
                <br>
                ----- Mail original -----
                <br>
                <blockquote type="cite">De: "Alan Bateman"
                  <a class="moz-txt-link-rfc2396E" href="mailto:Alan.Bateman@oracle.com"><Alan.Bateman@oracle.com></a>
                  <br>
                  À: "Remi Forax" <a class="moz-txt-link-rfc2396E" href="mailto:forax@univ-mlv.fr"><forax@univ-mlv.fr></a>, "Joe Wang"
                  <a class="moz-txt-link-rfc2396E" href="mailto:huizhe.wang@oracle.com"><huizhe.wang@oracle.com></a>
                  <br>
                  Cc: "nio-dev" <a class="moz-txt-link-rfc2396E" href="mailto:nio-dev@openjdk.java.net"><nio-dev@openjdk.java.net></a>,
                  "core-libs-dev"
                  <br>
                  <a class="moz-txt-link-rfc2396E" href="mailto:core-libs-dev@openjdk.java.net"><core-libs-dev@openjdk.java.net></a>
                  <br>
                  Envoyé: Vendredi 27 Avril 2018 13:34:12
                  <br>
                  Objet: Re: RFR (JDK11/NIO) 8201276: (fs) Add methods
                  to Files for
                  <br>
                  reading/writing a string from/to a file
                  <br>
                  On 27/04/2018 12:29, Remi Forax wrote:
                  <br>
                  <blockquote type="cite">I think that having a
                    readString that includes OS dependent line
                    separators is a
                    <br>
                    mistake,
                    <br>
                    Java does a great job to try to shield the developer
                    from the kind of things
                    <br>
                    that makes programs behave differently on different
                    platforms.
                    <br>
                    <br>
                    readString should subtitute (\r)?\n to \n otherwise
                    either people will do a call
                    <br>
                    replace() which is less efficient or will learn the
                    lesson the hard way.
                    <br>
                    <br>
                    raw string literal does the same substitution for
                    the same reason.
                    <br>
                    <br>
                  </blockquote>
                  Yes, there are several discussion points around this
                  and somewhat timely
                  <br>
                  with multi-string support.
                  <br>
                  <br>
                  One thing that I think Joe will need in this API is
                  some note to make it
                  <br>
                  clearer what the intended usage is (as I think the
                  intention is simple
                  <br>
                  cases with mostly single lines of text).
                  <br>
                  <br>
                  -Alan.
                  <br>
                </blockquote>
              </blockquote>
            </blockquote>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>