views:

525

answers:

12

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.
+3  A: 

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

Fire-Dragon-DoL
+3  A: 

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
+23  A: 

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
ah ha, very good. just the type of answer I was looking for.
wsanville
+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
+1 - Like some "XML" that I have to parse where the entities are numeric. Aarghle.
Rob
+1  A: 

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.

KeeperOfTheSoul
+2  A: 

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
wsanville
@wsanville: Anything to do with [[CDATA]], Unicode, Namespaces, Schemas, Processing Instructions.
Craig Trader
@wsanville: `<!-- did you know--this comment is invalid XML -->`
dtb
I did not, good call sir.
wsanville
A: 

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
A *week*? I don't know what it is you're doing wrong, but it's wrong.
Robert Rossney
+10  A: 

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.

Sander Pham
In the creation of a large document, there could be as many parentheses as in a Lisp program, but I have to admit this is the way I do it, too.
Gregory Higley
So *this* is called Linq-to-Xml! Golly.
Anton Tykhyy
@Gregory Higley: if you used StringBuilder you'd have a ton of < and >, Lisp by another name perhaps?
sixlettervariables
@sixlettervariables: I've heard that called "angle-bracket soup".
Gregory Higley
+2  A: 

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
+4  A: 

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
+3  A: 

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
A: 

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
+1  A: 

wsanville, it's attitudes like yours why we have to spend so many hours refactoring terrible code that is hard to maintain and impossible to reuse.

"To get from point A to point B quickly". And then you have to change something...

No thanks, not on my team.

stormianrootsolver
Unless you've been bitten by issues arising from XML generated via string concatenation, as I and many people I know have been, it might not be immediately obvious why that approach is dangerous.I recently found a junior developer checking in a piece of code that generated XML via string concatenation, but rather than berating him I held a 'brown bag' workshop working through the issues around that approach (without ever naming him as the inspiration for the session).This, I feel, is more constructive than simply balling someone out by yelling at "attitudes like yours..."
Ed Courtenay
In my opinion, a developer who does something like THIS has a more profound attitude problem. It is about ignorance and unwillingness to learn.
stormianrootsolver
hence the question, I wanted to learn why many consider this a poor approach. I got some great answers (cdonner pretty much sums it up), so now I've minimized any damage and I won't be doing this in the future :)
wsanville
wsanville, my suggestion is to always, always, always only use best practices and NEVER deviate even the slightest from that. :-)
stormianrootsolver
+1, agreed. If I didn't want to learn, I would have read the post from the above tread and said "I'm an all star and can get it right every time" and keep doing it the same way - not the case
wsanville
As Steve McConnell observed, the problem with quick and dirty solutions is that the dirtiness remains long after the quickness is but a dim, distant memory.
Robert Rossney