Errata

I am looking forward to reading this book, but I am somewhat stuck on the first recipe: "1.1 Managing Object End-of-Lifetime". Memory management is an important topic, so I want to make sure I get it right. I have 2 main questions:

1. On page 3, report?.Close() is called outside/after the if-statement which checks for disposing. Why is that? My thinking is that since report is a StreamWriter and StreamWriter is a managed object -- albeit one which internally manages an unmanaged resource -- the call to report.Close() should go inside the if(disposing) block. If the placement in the book is correct, what should run when IDisposable::Dispose() is called vs when the finalizer is called?

2. The call to report?.Close() uses the null-conditional member access operator. Why is that? My initial thinking was that we are checking in case the object referenced by report was collected prior to collecting the owning DeploymentProcess object, but that would depend on the garbage collector setting the DeploymentProcess::report instance variable to null upon collection. Does it do that? If not, it seems like the call to report?.Close() would cause undefined behavior since the report variable would still contain the reference to some memory which may have already been collected and possibly overwritten.

Thanks!

Note from the Author or Editor:
Hi Adam,

On question #1, `StreamWriter` is a type in the .NET Framework and seems like it should be treated as a managed object. I thought the same way and received feedback that made me see how it could be viewed another way. It revolves around the fact that the stream is for a file and the underlying implementation for Windows OS files is naturally via a Windows File Handle (https://docs.microsoft.com/en-us/windows/win32/fileio/file-handles), which is an unmanaged resource. The implications of this is that failing to release that resource could either prevent this or other apps from accessing that file or the OS runs out of file handles and an app or the whole system starts to crash.

Summarizing, the reason for the `if (disposing)` is to make sure that managed resources only dispose one time, preventing ObjectDisposedException. The way this example is written, the ObjectDisposedException scenario won't happen because of the call to `GC.SuppressFinalize`. If the calling code never called `Dispose`, the Finalizer would run and the call to `report?.Close()` would definitely happen. Since the object is a private field of this class, no-one else can Dispose it and we don't have to worry about the ObjectDisposedException. In another scenario, even if this object were passed in via constructor or some other possibility where it was set to null and the GC collected it, we have to look at the tradeoff of which is more evil - fail to release the file handle or run the weird risk of an intermittent ObjectDisposedException.

Generally, if I can convince enough developers to just do this when it's necessary, their code would be much better. If that's done, you'll be in a good place for this deep analysis of picking the least worse trade-off.

On question #2, I see your point and the null-conditional member access on `report?.Close()` isn't necessary in this example. Perhaps it was part of a habit that feeds my personal need for defensive programming (paranoia). That said, I've seen times in the past where this was necessary:

* If `report` was passed in a constructor, we don't know what other code did with it.
* If `report` was set to `null` inside of the class: maybe an elaborate disposal process that set it to `null`.
* Future maintenance that set it to `null` or an inadvertent bug.

Essentially, this prevents a `NullReferenceException` during the disposal process. Especially if the disposal path is through the Finalizer where this would be intermittent and hard for other developers to debug. It doesn't cause any problems because if `report` is null, nothing bad happens, others it calls `Close()` like normal

Tangentially, you're going to see me do things like this throughout the book because I have an ulterior motive - there's something about this that you'll see in the future. In this case, Chapter 3 has a recipe on a C# 8 feature called Nullable Reference types. In a nutshell (O'Reilly pun), Nullable Reference types improve the quality of your code by avoiding `NullReferenceExceptions`. Normally I give you better clues with mentioning a future recipe or adding a See Also reference. Now, I'm wondering if this would have been a good opportunity to do that in this case.

Thank you for your feedback and the opportunity to share my thoughts.

Regards,

Joe