Some time ago, I noted that the Process. property is an attractive nuisance because it is valid only on Process objects that you called Start on. You can’t just grab any old Process object and try to access its standard handles.
Others in the comments had their ideas on how to remove the confusion. Here’s mine. The principle is that the properties and methods of the Process object should be valid for all instances of the Process class. If a property or method is valid only conditionally, then either move it to a place that is accessible only if the condition is met, or get rid of it entirely if it adds no value.
The standard handles are the three properties that make sense only for Process objects that were created by the static Start method. There are also four methods related to those standard handles, as well as two events. Move them all to a new class, call it ProcessStartResult:
class ProcessStartResult
{
public Process Process { get; }
public System.IO.StreamWriter StandardInput { get; }
public System.IO.StreamWriter StandardOutput { get; }
public System.IO.StreamWriter StandardError { get; }
public void BeginOutputReadLine();
public void CancelOutputReadLine();
public event DataReceivedEventHandler? OutputDataReceived;
public void BeginErrorReadLine();
public void CancelErrorReadLine();
public event DataReceivedEventHandler? ErrorDataReceived;
}
Change the signature of all the overloads of the Start method so that they return a ProcessStartResult instead of a Process. Now it is impossible to do anything with the standard handles from a process you didn’t start: If you didn’t start the process, then you don’t have a ProcessStartResult. This removes the confusion that existed in the original attempt to have a process read from its own standard output.
This follows a principle I wrote about earlier: To force the developer to do things in a certain order, make the second step dependent on something produced by the first step. In this case, we want to force the developer to call Start before they use the standard handles, so we put the members related to the standard handles on a thing that you can obtain only by calling Start.
Next, remove the StartInfo property entirely. It serves two purposes:
- Prior to calling the
Startmethod, it provides a convenient pre-madeProcessStartInfo. - After calling the
Startmethod, it holds a copy of the parameters that you passed to theStartmethod.
The first purpose is just to cover for people who are too lazy to write the new keyword. So don’t be lazy. Write new ProcessStartInfo().
The second purpose doesn’t tell you anything you don’t already know, since you are the one who passed the parameters to the Start method in the first place. If they are so important to you, you can save them yourself.
Removing the StartInfo avoids confusion over whether the properties in it describe the process you want to start, or whether they describe a process that has already started. (And often, it describes neither!)
I think that takes care of the largest source of confusion over the proper use of the Process class.
I’ve found that managing state via types works great in practice! Most of my experience with this technique has been performance sensitive CPU-bound C++, but I’ve tried it on a microcontroller and I’ve tried it in C# webdev and it seems to generalize there as well.
Would it have been simpler to make PSI.RedirectStandardOutput() into a method that returns a StreamReader? (or some wrapper to either throw, or block, until the underlying process is started)
I can’t think of a scenario to want to start a child proc with redirected stdout, but then not actually intend to consume it.
.NET Team work on redesign of System.Diagnostics.Process API for upcoming .NET 11. Since .NET is open-source, you can trace (or even contribute opinions and/or code) at https://github.com/dotnet/runtime/issues/125838 and related issues/discussions.
While I agree that Process is a pain to work with, although nothing stops someone from creating their own wrapper, and the principle you recommend is good from a theoretical point of view it breaks down in more real world situations. Early in the framework development the recommendation was to keep the surface area small. The larger the surface area of an API the harder it can be to comprehend and use. There are entire sets of helper functions that could be added to core types but the justification for not doing that is, again, surface area.
With the principle...
Turns out in trying to design this thing, Windows is the weirdo.
Why? Windows permits waiting for arbitrary processes to exit. Other operating systems do not.
So, WaitForExit() and ExitCode are properties of ProcessStartResult, not Process.
I wrote my own replacement Process class. Probably the only thing I didn’t bother adding support for was the standard handles.
I say that the other operating systems are using a flawed design not fit for purpose.
Who is responsible for disposing the Process object in this design? Having to “using” a property seems odd.
var startResult = …; using (startResult.Process) { // Process is disposed after this block }To summarize : make the StartProcess functional instead of object oriented...
If we go that path...
Strictly speaking, the StartProcessResult should itself be split in multiple cases, as the StandardInput/Output/Error are only available if you redirect the standard streams... If I do not redirect stdin, then there should be no way for me to access the StandardInput stream. The redirection can no longer be a simple boolean that you switch.
Or at least make the streams nullable. In which case the BeginRead methods should themselves be accessible only conditionally.
By the way, I'm pretty sure StandardError and StandardOutput are StreamReaders, not StreamWriters.
Obviously, you need to dispose of the ProcessStartResult itself…