|
|
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.
|
|
|
|
|
Member 10277807 wrote: I'd like to see the let keyword 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.
let has 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
|
|
|
|
|
I can't do this either
const DateTime tick = new DateTime(2001, 1, 1);
- Pete
|
|
|
|
|
Of course, you would use readonly for that. I happen to agree with you that const should be allowed in these cases. The readonly verb is an admitted hack by the compiler writers. After all, string is an object (i.e. non-primitive) and you can use const with it.
#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 because const value must be known at compile-time. Instantiating DateTime requires code execution, which will only be done at run-time.
For that scenario you have the readonly , a run-time constant (intialized once and never changes).
You can only use const on variables that can be represented by literals.
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
|
|
|
|
|
pherschel wrote: Is it me or do you get confused by this? Why can't I say
const DateTime today = DateTime.Now;
Constants aren't variables, they just look like them, they are aids for the compiler, they don't exist at run-time. When you write
const int x = 5;
int y = 2;
int z = y + x;
bool b = z <= x;
what gets compiled is this
int y = 2;
int z = y + 5;
bool b = z <= 5;
The compiler replaces all instances of "x" with the constant value. If you could define x as DateTime.Now then what would be compiled? If it literally replaced "x" with "DateTime.Now" everywhere it appears then you almost certainly would not get the result you desire. If it replaced DateTime.Now with the date of compilation then that wouldn't work either. What you really want is a read-only variable, and that's why we have read only variables and constants. You have to understand what they are and use them appropriately, that's not a failing of .net.
This is also why you can't make objects const.
const Person p = new Person();
p.FirstName = "John";
p.Age = 33;
Console.WriteLine (p);
pherschel wrote: For properties, why can't I hide the worker variable for the rest of the class?
Use the short-hand version
int PageNbr { get; set; }
Again it's simply a case of understanding the difference and knowing when and where to use the appropriate solution.
pherschel wrote: For destructors, why can't you give me option to destroy right away?
Because .net is better at memory management than you are and if you leave .net to do your memory management you'll get code that performs well, if you try and do your own memory management you get code that performs badly. For one it means the code is harder for the compiler to optimise as the compiler likes to move your code around for the best overall result but when you have in-line memory management you lesser the optimiser's abilities. Also memory deallocation is expensive. If you could free your objects immediately you probably would (otherwise why would you want this feature?) and that will result in a not-insignificant performance hit. By leaving this aspect to .net it can clear the memory down at a time better suited, like when your app is idling, or some other conditions are met. That way the performance hit is not noticeable, it discretely happens in the background.
pherschel wrote: If you open a file and a DB you have to nest usings before you even get started doing some work
You don't have to nest usings at all
using (This x = new This())
using (That y = new That())
{
}
|
|
|
|
|
F-ES Sitecore wrote: Because .net is better at memory management than you are and if you leave .net to do your memory management you'll get code that performs well, if you try and do your own memory management you get code that performs badly. Well, you could jump outside of the CLR and allocate memory directly such as
unsafe
{
[DllImport(@"C:\Windows\System32\kernal32.dll")
public static extern void * HeapAlloc(void * procHeap, UInt32 dword, UInt32 size);
} Not saying that this is a good thing but it's possible and pretty much defeats the purpose of the CLR and GC. Not to mention memory management is one of the most challenging aspects of computer programing. If it weren't for .Net, my programs would most likely leak memory all over the place (my C and C++ KungFu is not strong).
if (Object.DividedByZero == true) { Universe.Implode(); }
Meus ratio ex fortis machina. Simplicitatis de formae ac munus. -Foothill, 2016
|
|
|
|
|