0

I'm imagining that this will come down to personal preference but which way would you go?

StringBuilder markup = new StringBuilder();

foreach (SearchResult image in Search.GetImages(componentId))
{
    markup.Append(String.Format("<div class=\"captionedImage\"><img src=\"{0}\" width=\"150\" alt=\"{1}\"/><p>{1}</p></div>", image.Resolutions[0].Uri, image.Caption));
}

LiteralMarkup.Text = markup.ToString();

Vs.

StringBuilder markup = new StringBuilder();

foreach (SearchResult image in Search.GetImages(componentId))
{
    markup.Append(String.Format(@"<div class=""captionedImage""><img src=""{0}"" width=""150"" alt=""{1}""/><p>{1}</p></div>", image.Resolutions[0].Uri, image.Caption));
}

LiteralMarkup.Text = markup.ToString();

Or should I not be doing this at all and using the HtmlTextWriter class instead?

EDIT: Some really good suggestions here. We are on 2.0 framework so LINQ is not available

Nick Allen
  • 11,970
  • 11
  • 45
  • 58
  • How come you can't just use ASPX? – Neil Barnwell Feb 26 '09 at 15:45
  • Always use HtmlTextWriter to write HTML, and abstract what you're writing from the logic that drives it. – Rex M Feb 26 '09 at 15:48
  • Whichever method you use, you need to Server.HtmlEncode() the URI and Caption before dumping it into the string or you will have HTML-injection holes leading to potential XSS. Any ‘good’ HTML templating system will do that for you... but note not all are ‘good’ ;-) – bobince Feb 26 '09 at 17:46
  • Using VS2008, you can write LINQ code and compile it to target the 2.0 framework. – Robert Rossney Feb 26 '09 at 19:41

5 Answers5

3

Me personally I would opt for the first option. Just to point out also, a quick code saving tip here. you can use AppendFormat for the StringBuilder.

EDIT: The HtmlTextWriter approach would give you a much more structured result, and if the amount of HTML to generate grew, then this would be an obvious choice. OR the use of an HTML file and using it as a template and maybe string replacements

ASP.NET User Control are another good choice for templating.

REA_ANDREW
  • 10,666
  • 8
  • 48
  • 71
  • OMG thanks for that tip, If I had a £1 for every time I wrote sb.Append(String.Format()) .... – Ian G Feb 26 '09 at 15:47
3

Another vote for "AppendFormat". Also, for the sake of the server code I might put up with single quotes here to avoid the need to escape anything:

StringBuilder markup = new StringBuilder();

foreach (SearchResult image in Search.GetImages(componentId))
{
    markup.AppendFormat(
        "<div class='captionedImage'><img src='{0}' width='150' alt='{1}'/><p>{1}</p></div>", 
        image.Resolutions[0].Uri, image.Caption
      );
}

LiteralMarkup.Text = markup.ToString();

Finally, you may also want an additional check in there somewhere to prevent html/xss injection.

Another option is to encapsulate your image in a class:

public class CaptionedHtmlImage
{  
    public Uri src {get; set;};
    public string Caption {get; set;}

    CaptionedHtmlImage(Uri src, string Caption)
    {
        this.src = src;
        this.Caption = Caption;
    }

    public override string ToString()
    {
        return String.Format(
            "<div class='captionedImage'><img src='{0}' width='150' alt='{1}'/><p>{1}</p></div>"
            src.ToString(), Caption
          );
    }
}

This has the advantage of making it easy to re-use and add features to the concept over time. To get real fancy you can turn that class into a usercontrol.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
2

I'd prefer:

StringBuilder markup = new StringBuilder();
string template = "<div class=\"captionedImage\"><img src=\"{0}\" width=\"150\" alt=\"{1}\"/><p>{1}</p></div>";

foreach (SearchResult image in Search.GetImages(componentId))
{
   markup.AppendFormat(template,image.Resolutions[0].Uri, image.Caption);
}

LiteralMarkup.Text = markup.ToString();

As mentioned in another answer, using AppendFormat().

Take a look at SharpTemplate for html templating, it makes it all a lot easier to read even for small amounts of HTML.

Chris S
  • 64,770
  • 52
  • 221
  • 239
  • @Chris +1 from me for the SharpTempate link. I have been looking for something like this for a while! Great Post. Cheers!! :-) – REA_ANDREW Feb 26 '09 at 15:49
1

I would prefer doing this in aspx using the ListView Control which exactly is made for this purpose. So your code will remain readable and is seperated (no HTML in the C# Code). With your approach you will get no compilte time XHTML validation warnings.

Andreas
  • 41
  • 3
1

I have made some helper classes to create html controls, so my code would look like this:

foreach (SearchResult image in Search.GetImages(componentId)) {
  ContainerMarkup.Controls.Add(
    Tag.Div.CssClass("captionedImage")
    .AddChild(Tag.Image(image.Resolutions[0].Uri).Width(150).Alt(Image.Caption))
    .AddChild(Tag.Paragraph.Text(Image.Caption)));
}

At first it might not look simpler, but it's easy to work with, creates controls that does proper html encoding of the values, and there is no string escaping issues.

Guffa
  • 687,336
  • 108
  • 737
  • 1,005