Hi Roger, Thank you for reviewing it! I've prepared another version.
diff --git a/src/java.base/unix/classes/java/lang/ProcessImpl.java b/src/java.base/unix/classes/java/lang/ProcessImpl.java --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java @@ -630,6 +630,21 @@ return !hasExited; } + /** + * The {@code toString} method for class {@code ProcessImpl} + * returns a string consisting of the native process ID of the process, + * exit code of the process and the status of the process. + * + * @return a string representation of the object. + */ + @Override + public String toString() { + return new StringBuilder("ProcessImpl{pid=").append(pid) + .append(", exitcode=").append(exitcode) + .append(", hasExited=").append(hasExited) + .append("}").toString(); + } + private static native void init(); static { On Mon, 9 May 2016 at 16:50 Roger Riggs <roger.ri...@oracle.com> wrote: > Hi Andrey, > > Since toString is a public method it needs to provide complete javadoc > specification > as providing values for debugging information. ToString methods > typically directly > reflect the state of the fields of the object. > > Why should the exitValue have the value of the pid if the process is not > alive? > > Also, the if toString is providing the exitValue then it should be > identified as "exitValue", not exitCode. > the isAlive value should be identified as "isAlive". > The implementation class should not be exposed. (remove > this.getClass().getSimpleName()). > > The performance of what seems like a simple toString method is not going > to be great > because of the native calls necessary to determine if the process is > alive (called twice) > and in the case of exitValue to handle the IllegalStateException. > > Some of the overhead could be avoided, by implementing toString in each > of the ProcessImpl.java > files where the current state is known more conveniently with less > overhead. > > $.02, Roger > > > On 5/8/2016 2:47 PM, Andrey Dyachkov wrote: > > Hello, > > > > I have added toString() method in Process.java. > > > > diff --git a/src/java.base/share/classes/java/lang/Process.java > > b/src/java.base/share/classes/java/lang/Process.java > > --- a/src/java.base/share/classes/java/lang/Process.java > > +++ b/src/java.base/share/classes/java/lang/Process.java > > @@ -548,5 +548,16 @@ > > return toHandle().descendants(); > > } > > > > + @Override > > + public String toString() { > > + boolean isAlive = this.isAlive(); > > + return new > > StringBuilder(this.getClass().getSimpleName()).append("[") > > + .append("running=").append(isAlive).append(", ") > > + .append(isAlive ? "pid=" : "exitCode=") > > + .append(isAlive ? this.getPid() : > this.exitValue()) > > + .append("]") > > + .toString(); > > + } > > + > > > > } > > -- With great enthusiasm, Andrey