|
True enough. I'd love to banish usings somehow. I want to use a create & use an class, not manage it's lifetime.
- Pete
|
|
|
|
|
You don't have to use the "using statement". You can create an instance of the object and dispose of it manually when you are done.
|
|
|
|
|
usings are only necessary when you need to know deterministically that a resource has been freed.
#SupportHeForShe
Government can give you nothing but what it takes from somebody else. A government big enough to give you everything you want is big enough to take everything you've got, including your freedom.-Ezra Taft Benson
You must accept 1 of 2 basic premises: Either we are alone in the universe or we are not alone. Either way, the implications are staggering!-Wernher von Braun
|
|
|
|
|
That's not entirely true. Look at streams.
One would think this is correct.
using (var outerStream = new MemoryStream(someData))
{
using (var innerStream = new TextReader(outerStream))
{
}
} However, the proper way is this
var outerStream = new MemoryStream(someData);
using (var innerStream = new TextReader(outerStream)
{
} This is because the a stream will dispose of the underlying streams when you call Stream.Dispose(). I had code analysis bark at me all the time until I figured this one out.
if (Object.DividedByZero == true) { Universe.Implode(); }
Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
|
|
|
|
|
I was always under the impression that any IO stuff should be done in the "using" statement. Once the memory stream instance is not needed then it will be disposed, no need to kill it manually.
Can you please show be some supporting evidence to support your later example, because I don't know that to be entirely true.
|
|
|
|
|
That little path of discovery started when I started learning the Cryptography namespace. This little code snippet (copied directly from MSDN) was getting flagged with CA2202 during code analysis.
using (MemoryStream msDecrypt = new MemoryStream(cipherText))
{
using (CryptoStream csDecrypt = new CryptoStream(msDecrypt, decryptor, CryptoStreamMode.Read))
{
using (StreamReader srDecrypt = new StreamReader(csDecrypt))
{
plaintext = srDecrypt.ReadToEnd();
}
}
}
This really boggled me as this is Microsoft example code being flagged as incorrect so I began to dig. Turns out there is still a lively debate about this since the documentation is a little unclear in this area. Refactoring it to the following stopped the code analysis from flagging the code.
MemoryStream memStream = new MemoryStream(data);
CryptoStream decStream = new CryptoStream(memStream, decryptor, CryptoStreamMode.Read);
using (StreamReader reader = new StreamReader(decStream))
{
decryptedValue = Encoding.UTF8.GetBytes(reader.ReadToEnd());
}
Research into this led me to this one little line in this MSDN article: StreamReader Constructor.
The StreamReader object calls Dispose() on the provided Stream object when StreamReader.Dispose is called.
This reads that when you close certain classes of streams, they also close the streams that underlie them as well.
TL;DR
Not all IDisposable classes behave the same way, event in Microsoft code, and the using statement isn't always the correct way.
if (Object.DividedByZero == true) { Universe.Implode(); }
Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
|
|
|
|
|
|
Foothill wrote: However, the proper way is this
var outerStream = new MemoryStream(someData);
using (var innerStream = new TextReader(outerStream)
{
} This is because the a stream will dispose of the underlying streams when you call Stream.Dispose(). I had code analysis bark at me all the time until I figured this one out.
No, it's not: if "new TextReader(outerStream)" throws you get an unclosed outerStream.
The code analysis tool you're using is broken unless it can prove that the TextReader constructor never throws, and I really doubt that 1) it's doing that analisys and 2) that it's good practice to encourage not doing the outer using because in one specific case it's guaranteed the inner stream constructor doesn't throw.
|
|
|
|
|
Sorry, TextReader was a bad example. I typed it for speed. I went into more detail here in this response[^]. There are, however, certain stream classes that implement IDisposable in which the disposal pattern goes further and disposes of underlying streams.
The code analysis is built into VS 2015 and I run it with all the rules turned on so I don't think it's broken. It was detecting MS .Net code that was going to behave contrary to preconceptions of IDisposable objects.
if (Object.DividedByZero == true) { Universe.Implode(); }
Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
|
|
|
|
|
I agree that TextReader is a bad example for other reasons: probably the constructor doesn't throw.
And I saw your other response, and it doesn't change things: it clarifies that Dispose does indeed call close on the outerStream, but it doesn't say a thing about what happens if the constructor throws.
My point is that with:
var outerStream = new AStream();
using (var innerStream = new AnotherStream(outerStream)) {...} If "new AnotherStream(outerStream)" throws you get to hold the undisposed baby as innerStream.Dispose() never gets called and the outerStream isn't closed.
Can't comment on VS code analysis, as I only use Resharper, but just for fun, trying VS2015 code analysis (all rules on) on this code:
{
var outerStream = new MyStream();
using (var innerStream = new MyStream(outerStream)) {
bool i = innerStream.CanRead;
Console.WriteLine("Read: " + i);
}
}
{
using (var outerStream = new MyStream())
{
using (var innerStream = new MyStream(outerStream))
{
bool i = innerStream.CanRead;
Console.WriteLine("Read: " + i);
}
}
} [Line 59] Warning CA2000 In method 'Program.Main(string[])', call System.IDisposable.Dispose on object 'outerStream' before all references to it are out of scope.
[Line 73] Warning CA2202 Object 'outerStream' can be disposed more than once in method 'Program.Main(string[])'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.
Bit schizophrenic: in the first case complains that Dispose isn't called and on the second complains that it's called twice
Edit:
- Moved Line 73 one line up...
- VS code analysis agrees with me: the first warning says that with only one using it's not guaranteed that the outer stream gets closed.
-(Re)thinking on this, VS code analysis is not smart enough: it knows that innerStream.Dispose() calls outerStream.Dispose() but doesn't know that Stream.Dispose() is (should always be) idempotent and can get called any number of times.
modified 8-Jun-17 15:27pm.
|
|
|
|
|
I can see your point but accounting for errors in Stream constructors, which you can catch, isn't close to the point I was trying to make. I was just implying that it's generally a good idea to use using but is not always a straight forward case such as when using streams that one would assume derive from System.IO.Stream based on code symantics. What sets StreamReader and StreamWriter apart is that, while they behave exactly like a stream, they do not derive from System.IO.Stream. They derive from System.TextReader which in turn derives directly from System.MarshalByRefObject. So here we have two classes mimicking the behavior of an entire branch of classes but have subtle differences in implementation. Since most people would assume that StreamReader derived from IO.Stream, it can cause confusion when CA2202 messages warn that you're disposing of a object twice even thought you are actually following recommended best practices.
if (Object.DividedByZero == true) { Universe.Implode(); }
Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
|
|
|
|
|
Hmm... using the example on your other post (changed just enough to make the code compile):
byte[] data = new byte[1000];
MemoryStream memStream = new MemoryStream(data);
CryptoStream decStream = new CryptoStream(memStream, SHA1.Create(), CryptoStreamMode.Read);
using (StreamReader reader = new StreamReader(decStream)) {
byte[] decryptedValue = Encoding.UTF8.GetBytes(reader.ReadToEnd());
Console.WriteLine(decryptedValue);
} We get a (for me accurate) CA2000 warning: object 'memStream' is not disposed along all exception paths.
And... what's this, no warning about an undiposed decStream??? Let me check:
SHA1 sha1 = SHA1.Create();
byte[] data = new byte[1000];
using (MemoryStream memStream = new MemoryStream(data)) {
CryptoStream decStream = new CryptoStream(memStream, sha1, CryptoStreamMode.Read);
StreamReader reader = new StreamReader(decStream);
byte[] decryptedValue = Encoding.UTF8.GetBytes(reader.ReadToEnd());
Console.WriteLine(decryptedValue);
} No undisposed warnings with VS about this.
Was going to say I could agree with this code, but no, even though it passes VS code analysis I disagree even more with it. Who can say what goes on in all the undisposed inner streams in the case of an exception?
For me:
a) (repeat from my other post) It seems that while VS2015 (and 2017, just tested) code analysis is smart enough to know that innerStream.Dispose calls outerSteam.Dispose it's not smart enough to know that Dispose in streams should be idempotent and should be able to be called several times.
b) It's really strange there being a best practice that doesn't ensure every stream is closed in all code paths, including exceptions.
|
|
|
|
|
Yes. I can say that the code does not handle disposing of the streams along all exception paths but, for some context, the only time a stream constructor will fail is when you pass it bad arguments or bad file paths. In well tested code, the only time a stream constructor should fail is for StackOverflowException or OutOfMemoryException and your program is pretty much hosed at that point anyways.
if (Object.DividedByZero == true) { Universe.Implode(); }
Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
|
|
|
|
|
That's really really odd - is there any example of a Dispose() method throwing an ObjectDisposedException if called twice? Because if so, that's going against the guidelines for implementing a Dispose method. Dispose methods should never throw for exactly this reason (among others), so the Line 73 warning seems rather odd.
|
|
|
|
|
pherschel wrote: const DateTime today = DateTime.Now;
What happens if you're still using your application after midnight?
pherschel wrote: For properties, why can't I hide the worker variable for the rest of the class?
public int PageNbr
{
int _worker = 9;
get { return _worker;}
set { _worker = value; }
}
Something doesn't look right with this, but assuming I correctly guessed what you're trying to do, you can initialize auto properties at declaration now and not have a backing variable at all.
public int PageNbr{ get; set; } = 9;
Did you ever see history portrayed as an old man with a wise brow and pulseless heart, waging all things in the balance of reason?
Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful?
--Zachris Topelius
Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies.
-- Sarah Hoyt
|
|
|
|
|
pherschel wrote: I can see readonly for parameters and such,
That's actually one of the things I love about Swift[^]: params are by default readonly
cheers
Chris Maunder
|
|
|
|
|
That's actually a VERY good idea. I.e. if you wanted some by-value parameter and be able to change its value inside the function, decorate it with something like volatile (or perhaps duplicated would be a more apt naming). Or to just make it sound similar to out and ref ... call it something like in, or dup, or copy, ...
This would allow for so much better optimisations across the board, while disallowing stupid things like assigning new values to by-value parameters unless you specifically state that's what you want.
|
|
|
|
|
Quote: Why can't I say
const DateTime today = DateTime.Now;
Because DateTime.Now is not a compile-time constant, it's a run time property which returns an non-constant value.
Suppose it did allow it: what value should be in today ?
The DatetIme when the app was started? When the assembly containing the code was loaded? When the class containing the constant value was statically initialized?
What would happen if two different classes (or worse assemblies) both declared the same value? Would they be the same? Should they be? How would the system decide for you?
That's the point of const vs readonly - the former is a compile time constant value, the later is a runtime constant value. That way, you have the choice for what exactly you want to do, rather than letting the system try to make up it's mind for you.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
|
OriginalGriff wrote: What would happen if two different classes (or worse assemblies) both declared the same value? Would they be the same? Should they be? How would the system decide for you?
Well no they because they would be separate instances (each class it's own), even if from the same or separate assemblies.
Sin tack
the any key okay
|
|
|
|
|
OriginalGriff wrote: That's the point of const vs readonly - the former is a compile time constant value, the later is a runtime constant value. While this is true, readonly is used for objects (except for string ) as a compiler hack because const ant (i.e. readonly ) objects are really pointers whose value varies at load time. The fact the compiler writers did it for string means they could have done it for any object. But, I'm not interested in the object's pointer, I'm interested in the object's const ant value.
#SupportHeForShe
Government can give you nothing but what it takes from somebody else. A government big enough to give you everything you want is big enough to take everything you've got, including your freedom.-Ezra Taft Benson
You must accept 1 of 2 basic premises: Either we are alone in the universe or we are not alone. Either way, the implications are staggering!-Wernher von Braun
|
|
|
|
|
TheGreatAndPowerfulOz wrote: The fact the compiler writers did it for string means they could have done it for any object.
That's not true, although string is an object, it's a special type of object. It's immutable and can be allocated both on the heap and the stack. Other object types are allocated on the heap exclusively but are not nativelly immutable.
Strings also have a mechanism called interning. By default const strings are interned for optimization purposes.
Having that said, string gets all kinds of special treatment. Remember that you can only declare a const string literal. For example:
const string _myString = String.Empty;
This also defeats the statement that other reference types could have the same treatment. As every reference type you want to use const with, would have to have a literal representation, so its value could be determined at compile-time. Like the string literal.
TheGreatAndPowerfulOz wrote: readonly is used for objects (except for string )
Value types can also be readonly.
Reinforcing what Original Griff said, the difference between readonly and const are as simple as one is a run-time constant and the other a compile-time constant. If a variable's value cannot be determined at compile-time, it cannot be a const .
To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson
Our heads are round so our thoughts can change direction - Francis Picabia
modified 8-Jun-17 8:00am.
|
|
|
|
|
What you say may be true, but const could have and should have been used for both situations.
#SupportHeForShe
Government can give you nothing but what it takes from somebody else. A government big enough to give you everything you want is big enough to take everything you've got, including your freedom.-Ezra Taft Benson
You must accept 1 of 2 basic premises: Either we are alone in the universe or we are not alone. Either way, the implications are staggering!-Wernher von Braun
|
|
|
|
|
TheGreatAndPowerfulOz wrote: could
Yes, could, but should it?
They behave differently and actually generate different IL code. The compiler could also determine that automatically, when generating IL code. But I don't think that's a good idea, but we will end up in another filosophical discussion. In my opinion this compiler behavior could generate those situations developers don't understand what's happening and why their code does not work as expected.
To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson
Our heads are round so our thoughts can change direction - Francis Picabia
|
|
|
|
|
Fabio Franco wrote: should it? Yes.
Fabio Franco wrote: my opinion Doubt it.
Fabio Franco wrote: another filosophical discussion That's what life is about.
#SupportHeForShe
Government can give you nothing but what it takes from somebody else. A government big enough to give you everything you want is big enough to take everything you've got, including your freedom.-Ezra Taft Benson
You must accept 1 of 2 basic premises: Either we are alone in the universe or we are not alone. Either way, the implications are staggering!-Wernher von Braun
|
|
|
|
|
I'd like to see the let keyword added to C# for this purpose, to replace var in cases where the binding will not be mutated. Swift makes this distinction, and F# uses let with this meaning (and let mutable for what C# calls var ).
It also just feels really good, like a benevolent ruler. The variable wants to have this value, and you just have to let it.
const is what JavaScript uses for non-mutable bindings, but in C# it already specifically refers to compile-time constants in C#. Also, const just doesn't feel as good as let ting a variable follow its heart.
|
|
|
|