The Daily WTF: Curious Perversions in Information Technology

MaxiTB · 2024-02-20 Reply Admin

This is managed C++, it's not a dialect, it is it's own standardized language and has little to do with C++. And I would call XML overspecified, it was in fact invented to replace overspecificed transport protocols with a simple generic one that only specifies syntax but not sematics rules like CORBA.

Now personally I would have implemented that logic in C#, because it doesn't make use of low level operations at all (that's where managed C++ shines, because you can mixup easily managed code with native code with little marshalling overhead while being able to implement critical paths basically on a platform dependent machine language level).

There's a ton of weird stuff in the code, starting with the use of this. You never ever use this except for indexers or constructors. Oddly enough while not using the old-school 'm_'+' s_' style, they use the other generally accepted '_' prefix style for member fields. So huh?

Now obviously this code was written by someone who has no experience in writing code at all:

StreamReader ^reader = gcnew StreamReader(file->OpenRead());

What's worse than one memory leak? Well, two in a line. Now you could argue that by default StreamReader disposes underlying unmanaged resources (ha, the irony), but one exception and you not only have the memory leak, you also block the file and system resources (on some platforms forever - well, until you restart the system).

Still, I have to give them props for using the XElement components instead of XmlElement components, so there's at least that and doing proper Path.Combine's; on the other hand they use types instead of keywords. Wait, what?

This code was properly written by two very different people at once haha

Addendum 2024-02-20 07:16: This code is so creepy.

String ^key = String::Empty;

Using the better performant interned string empty instead of the "" literal (when you use .net 5+ the compiler does the job finally), but again not using the keyword but the type.

colejohnson66 · 2024-02-20 Reply Admin

"Thanks guys, I really appreciate this!"

jeremypnet · 2024-02-20 Reply Admin

So we start by confirming that this object hasn't been disposed, and throwing an exception if it has.

That's a WTF by itself. In order to call a method on an object, we must surely have a reference to it, so why would the garbage collector collect it?

2024-02-20 Reply Admin

OH so much wrong with today's post and comments...

It is NOT "Managed C++" that came out with VS-2003 and was "it's own standardized language and has little to do with C++.".... As of VS-2005 there is a completely different thing which is C++/CLI. Released tiwh VS-2005, it is 100% compliant [yes, there were bugs over time, but they were remediated] with ISO C++ standards and approved extensibility points...
XML supports ordered elements, and if one had the correct XSD [schema] the children must appear in the appropriate order...

If there is a "Real WTF" it is that XML files were accepted by people without being provided a associated Schema [XSD] and without having a full "validate and reject" process at the front end...

2024-02-20 Reply Admin

The fact that it saves many times files on disk and just to read them again just after is also probably bad for performance... if the code works at all. My instinct is telling me it probably doesn't work in the smoothest and least buggy way imaginable.

mynameishidden · 2024-02-20 Reply Admin

If you decide to fix the problem with XSLT, now you have five problems.

dpm · 2024-02-20 Reply Admin

There's a lot going on here.

Fortunately the comments explain not only what is going on here, but why it had to be done that way. Kudos!

2024-02-20 Reply Admin

This is .net, so Dispose is its own thing. Tracing GCs free memory (and run destructors/finalizers) when they feel like it, which is unfortunate for more RAII things like file handles that should be closed promptly. IDisposable is the .net interface for dealing with that sort of thing: its single method Dispose is called when such cleanup should be performed, and language constructs like using blocks in C# know about it.

MaxiTB · 2024-02-20 Reply Admin

To be fair, a lot people are calling C++/CLI just managed C++ because it was obsolete the moment C++/CLI came out. Ironically the code should be valid for both, but maybe I missed something :-)

MaxiTB · 2024-02-20 Reply Admin

That's not what an IDisposable does. It implements a method that releases system resource which need to be released obviously ASAP, like file descriptor handles. You don't want to wait for GC to finally do this, in fact, it can especially in parallel scenarios lead to nasty race conditions or dead locks not doing so. This is for example a major issue in other languages which don't have a standardized way to handle disposing native system resources.

The thing that is wrong for this code is, that for example the line

StreamReader ^reader = gcnew StreamReader(file->OpenRead());

must look like this to avoid major issues when exception occur far beyond just a memory leak:

Stream^ stream = null;
Reader^ reader = null;
try
{
  stream = file->OpenRead();
  reader = gcnew StreamReader(stream);

  // Here goes the rest of the code as long as the reader is in use
}
finally
{
  if(reader != null) reader.Close();
  if(stream != null) stream.Close();
}

Addendum 2024-02-20 12:26: Obviously I did the C# thing here out of habit. In managed C++ you are using delete instead which will also called the Dispose method which does for streams obviously the same.

tom103 · 2024-02-20 Reply Admin

OMG, C++/CLI! I hadn't seen this in a long time

tom103 · 2024-02-20 Reply Admin

Disposed isn't the same as garbage collected. The Dispose method is intended to release unmanaged resources (e.g. file handles etc.). If the class is properly implemented, the destructor (which is called when the object is garbage collected) will call Dispose, but you can also call it explicitly beforehand. There are actually valid use cases for checking if an object is disposed, which is why the ObjectDisposedException class exists in the standard library.

2024-02-20

Comment held for moderation.

Watson · 2024-02-21 Reply Admin

Either .NET's regular expression syntax is a special snowflake in C++, or wonky anonymisation, or

			Regex ^isRecOrFld = gcnew Regex("^\\s+\\<[REC|FLD].*$");

makes me very sad.

Medinoc · 2024-02-21 Reply Admin

C++/CLI has no verbatim strings, this may be part of the problem.

2024-02-26

Comment held for moderation.

Merge the Files

Leave a comment on “Merge the Files”