25

In the thread What’s your favorite “programmer ignorance” pet peeve?, the following answer appears, with a large amount of upvotes:

Programmers who build XML using string concatenation.

My question is, why is building XML via string concatenation (such as a StringBuilder in C#) bad?

I've done this several times in the past, as it's sometimes the quickest way for me to get from point A to point B when to comes to the data structures/objects I'm working with. So far, I have come up with a few reasons why this isn't the greatest approach, but is there something I'm overlooking? Why should this be avoided?

  1. Probably the biggest reason I can think of is you need to escape your strings manually, and most new programmers (and even some experienced programmers) will forget this. It will work great for them when they test it, but then "randomly" their apps will fail when someone throws an & symbol in their input somewhere. Ok, I'll buy this, but it's really easy to prevent the problem (SecurityElement.Escape to name one).
  2. When I do this, I usually omit the XML declaration (i.e. <?xml version="1.0"?>). Is this harmful?
  3. Performance penalties? If you stick with proper string concatenation (i.e. StringBuilder), is this anything to be concerned about? Presumably, a class like XmlWriter will also need to do a bit of string manipulation...
  4. There are more elegant ways of generating XML, such as using XmlSerializer to automatically serialize/deserialize your classes. Ok sure, I agree. C# has a ton of useful classes for this, but sometimes I don't want to make a class for something really quick, like writing out a log file or something. Is this just me being lazy? If I am doing something "real" this is my preferred approach for dealing w/ XML.
Community
  • 1
  • 1
wsanville
  • 37,158
  • 8
  • 76
  • 101
  • Side note: The referred link on top of post has been removed from Stack Overflow for reasons of moderation. – T.M. Mar 02 '23 at 17:59

12 Answers12

30

You can end up with invalid XML, but you will not find out until you parse it again - and then it is too late. I learned this the hard way.

cdonner
  • 37,019
  • 22
  • 105
  • 153
  • +1 - Often it is the consumer of the broken XML who is left with the task of trying to find a workaround for the brokenness. THAT is why this gets the label of a "pet peeve"! – Stephen C Jun 14 '10 at 09:18
  • +1 - Like some "XML" that I have to parse where the entities are numeric. Aarghle. – Rob Jun 14 '10 at 12:19
15

I think readability, flexibility and scalability are important factors. Consider the following piece of Linq-to-Xml:

XDocument doc = new XDocument(new XDeclaration("1.0","UTF-8","yes"),
   new XElement("products", from p in collection
    select new XElement("product",
        new XAttribute("guid", p.ProductId), 
        new XAttribute("title", p.Title),
        new XAttribute("version", p.Version))));

Can you find a way to do it easier than this? I can output it to a browser, save it to a document, add attributes/elements in seconds and so on ... just by adding couple lines of code. I can do practically everything with it without much of effort.

S P
  • 4,615
  • 2
  • 18
  • 31
6

Actually, I find the biggest problem with string concatenation is not getting it right the first time, but rather keeping it right during code maintenance. All too often, a perfectly-written piece of XML using string concat is updated to meet a new requirement, and string concat code is just too brittle.

As long as the alternatives were XML serialization and XmlDocument, I could see the simplicity argument in favor of string concat. However, ever since XDocument et. al., there is just no reason to use string concat to build XML anymore. See Sander's answer for the best way to write XML.

Another benefit of XDocument is that XML is actually a rather complex standard, and most programmers simply do not understand it. I'm currently dealing with a person who sends me "XML", complete with unquoted attribute values, missing end tags, improper case sensitivity, and incorrect escaping. But because IE accepts it (as HTML), it must be right! Sigh... Anyway, the point is that string concatenation lets you write anything, but XDocument will force standards-complying XML.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
6

I wrote a blog entry back in 2006 moaning about XML generated by string concatenation; the simple point is that if an XML document fails to validate (encoding issues, namespace issues and so on) it is not XML and cannot be treated as such.

I have seen multiple problems with XML documents that can be directly attributed to generating XML documents by hand using string concatenation, and nearly always around the correct use of encoding.

Ask yourself this; what character set am I currently encoding my document with ('ascii7', 'ibm850', 'iso-8859-1' etc)? What will happen if I write a UTF-16 string value into an XML document that has been manually declared as 'ibm850'?

Given the richness of the XML support in .NET with XmlDocument and now especially with XDocument, there would have to be a seriously compelling argument for not using these libraries over basic string concatenation IMHO.

Ed Courtenay
  • 520
  • 5
  • 14
5

I think that the problem is that you aren't watching the xml file as a logical data storage thing, but as a simple textfile where you write strings.

It's obvious that those libraries do string manipulation for you, but reading/writing xml should be something similar to saving datas into a database or something logically similar

Francesco Belladonna
  • 11,361
  • 12
  • 77
  • 147
3

If you need trivial XML then it's fine. Its just the maintainability of string concatenation breaks down when the xml becomes larger or more complex. You pay either at development or at maintenance time. The choice is yours always - but history suggests the maintenance is always more costly and thus anything that makes it easier is worthwhile generally.

Preet Sangha
  • 64,563
  • 18
  • 145
  • 216
2

You need to escape your strings manually. That's right. But is that all? Sure, you can put the XML spec on your desk and double-check every time that you've considered every possible corner-case when you're building an XML string. Or you can use a library that encapsulates this knowledge...

dtb
  • 213,145
  • 36
  • 401
  • 431
2

Another point against using string concatenation is that the hierarchical structure of the data is not clear when reading the code. In @Sander's example of Linq-to-XML for example, it's clear to what parent element the "product" element belongs, to what element the "title" attribute applies, etc.

Todd Owen
  • 15,650
  • 7
  • 54
  • 52
1

As you said, it's just awkward to build XML correct using string concatenation, especially now you have XML linq that allows for simple construction of an XML graph and will get namespaces, etc correct.

Obviously context and how it is being used matters, such as in the logging example string.Format can be perfectly acceptable.

But too often people ignore these alternatives when working with complex XML graphs and just use a StringBuilder.

Chris Chilvers
  • 6,429
  • 3
  • 32
  • 53
1

The main reason is DRY: Don't Repeat Yourself.

If you use string concat to do XML, you will constantly be repeating the functions that keep your string as a valid XML document. All the validation would be repeated, or not present. Better to rely on a class that is written with XML validation included.

Carlos
  • 5,991
  • 6
  • 43
  • 82
0

I've always found creating an XML to be more of a chore than reading in one. I've never gotten the hang of serialization - it never seems to work for my classes - and instead of spending a week trying to get it to work, I can create an XML file using strings in a mere fraction of the time and write it out.

And then I load it in using an XMLReader tree. And if the XML file doesn't read as valid, I go back and find the problem within my saving routines and corret it. But until I get a working save/load system, I refuse to perform mission-critical work until I know my tools are solid.

I guess it comes down to programmer preference. Sure, there are different ways of doing things, for sure, but for developing/testing/researching/debugging, this would be fine. However I would also clean up my code and comment it before handing it off to another programmer.

Because regardless of the fact you're using StringBuilder or XMLNodes to save/read your file, if it is all gibberish mess, nobody is going to understand how it works.

Jeffrey Kern
  • 2,024
  • 20
  • 40
0

Maybe it won't ever happen, but what if your environment switches to XML 2.0 someday? Your string-concatenated XML may or may not be valid in the new environment, but XDocument will almost certainly do the right thing.

Okay, that's a reach, but especially if your not-quite-standards-compliant XML doesn't specify an XML version declaration... just saying.

catfood
  • 4,267
  • 5
  • 29
  • 55