[PATCH] JDK-8155102: Process.toString could include pid, isAlive, exitStatus

[Roger Riggs]

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();
> +    }
> +
>
>   }