c# Const vs. Readonly, Scope, Usings
-
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
-
With C# getting to version 7+ I wish I could have some basic improvments. Is it me or do you get confused by this? Why can't I say
const DateTime today = DateTime.Now;
I can see readonly for parameters and such, but I would be happy using const there too
void Doit(const MyObj arg) ...
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; }
}For destructors, why can't you give me option to destroy right away? I hate disposing with all its using code bloat. How about a free keyword on a variable or something to get me out of the business of resource management. If you open a file and a DB you have to nest usings before you even get started doing some work! Or maybe I'm missing something?
- Pete
These are probably a good idea. Though semantic-wise I'd actually want it to be a readonly instead. In C++ const is used to give the compiler a hint so it knows this parameter wont get changed inside the function, so a copy of it need not go onto its stack (just a direct memory link instead). But even there I'd actually want to rename it to something like readonly instead, const just confuses the hell out of me (is it only a param calculated at compile time, why then not just a normal const in the first place and do away with the param entirely?)
-
With C# getting to version 7+ I wish I could have some basic improvments. Is it me or do you get confused by this? Why can't I say
const DateTime today = DateTime.Now;
I can see readonly for parameters and such, but I would be happy using const there too
void Doit(const MyObj arg) ...
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; }
}For destructors, why can't you give me option to destroy right away? I hate disposing with all its using code bloat. How about a free keyword on a variable or something to get me out of the business of resource management. If you open a file and a DB you have to nest usings before you even get started doing some work! Or maybe I'm missing something?
- Pete
Here you may be onto something. I'd imagine getting something like this working at present would be a form of making automatic struct types for each property - incorporating their default properties with a get set as defined in code. Perhaps including it into the spec could be done more efficiently. It should definitely not be enforced though. In some cases the rest of the class does need to see the underlying data without needing to go through the accessor methods. Which in turn brings up another issue in C# ... the inability to have default properties like you can in VB-Net. In C# your closest match is to make implicit type-casting operator overloads.
-
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.
-
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
I'd like to see the
letkeyword added to C# for this purpose, to replacevarin cases where the binding will not be mutated. Swift makes this distinction, and F# usesletwith this meaning (andlet mutablefor what C# callsvar). It also just feels really good, like a benevolent ruler. The variable wants to have this value, and you just have toletit.constis what JavaScript uses for non-mutable bindings, but in C# it already specifically refers to compile-time constants in C#. Also,constjust doesn't feel as good asletting a variable follow its heart. -
I'd like to see the
letkeyword added to C# for this purpose, to replacevarin cases where the binding will not be mutated. Swift makes this distinction, and F# usesletwith this meaning (andlet mutablefor what C# callsvar). It also just feels really good, like a benevolent ruler. The variable wants to have this value, and you just have toletit.constis what JavaScript uses for non-mutable bindings, but in C# it already specifically refers to compile-time constants in C#. Also,constjust doesn't feel as good asletting a variable follow its heart.Member 10277807 wrote:
I'd like to see the
letkeyword added to C#That would actually be pretty cool. It already exists with Linq, but it would a nice addition to the language itself, at least for value types.
lethas a functional nature and C# has been slowly adopting some of the functional programming features. I can see it come to C# soon enough.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
-
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))
{
// do something
}
}However, the proper way is this
var outerStream = new MemoryStream(someData);
using (var innerStream = new TextReader(outerStream)
{
// do something
}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
Foothill wrote:
However, the proper way is this
var outerStream = new MemoryStream(someData);
using (var innerStream = new TextReader(outerStream)
{
// do something
}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.
-
Foothill wrote:
However, the proper way is this
var outerStream = new MemoryStream(someData);
using (var innerStream = new TextReader(outerStream)
{
// do something
}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
-
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(); // line 59
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 73
}[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.
-
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(); // line 59
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 73
}[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.
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
usingbut 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
-
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(); // line 59
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 73
}[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.
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.
Blog: [Code Index] By Mike Marynowski | Business: Singulink
-
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
usingbut 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.
-
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
StackOverflowExceptionorOutOfMemoryExceptionand 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
-
With C# getting to version 7+ I wish I could have some basic improvments. Is it me or do you get confused by this? Why can't I say
const DateTime today = DateTime.Now;
I can see readonly for parameters and such, but I would be happy using const there too
void Doit(const MyObj arg) ...
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; }
}For destructors, why can't you give me option to destroy right away? I hate disposing with all its using code bloat. How about a free keyword on a variable or something to get me out of the business of resource management. If you open a file and a DB you have to nest usings before you even get started doing some work! Or maybe I'm missing something?
- Pete