• (nodebb)

    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.

  • (nodebb)

    "Thanks guys, I really appreciate this!"

  • (nodebb)

    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?

  • TheCPUWizard (unregistered)

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

    1. 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...

    2. 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...

  • Sauron (unregistered)

    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.

  • (nodebb)

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

  • (nodebb)

    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!

  • Kythyria (unregistered) in reply to jeremypnet

    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.

  • (nodebb) in reply to jeremypnet

    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.

  • (nodebb)

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

  • (nodebb) in reply to jeremypnet

    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.

  • Officer Johnny Holzkopf (unregistered)

    What about the sequence point consideration and postfix increment? At one point, there is 'int i = 1;', and later on, we find 'splitted->Save(Path::Combine(this->_sourceDir->FullName, this->docPrefix + "" + i++ + ".xml"));' where i++ should (?) be evaluated before the actual function call, so it would end as 'docPrefix + "" + 2 + ".xml"' at the first invocation, and then increment from there on - no 'blah_1.xml' file. Is that a case for the manager of managed C++? Or doctor Prefix? FIles definitely are not 1st class citizens here...

  • (nodebb)

    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.

  • (nodebb) in reply to Watson

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

Leave a comment on “Merge the Files”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article