7
\$\begingroup\$

Iwas asked to give more context. So here we go.

We want to create some HTML in code behind to add an "add" and a "refresh" button to a grid view header column. Approach #1 does this using string concatination to produce the necessary HTML, whereas approach #2 uses .NET objects to create exactly the same HTML.

This time I'm posting the complete classes:

Approach #1

class AddBtnTemplate : ITemplate {    StatRefTimeGrid Parent;    public AddBtnTemplate(StatRefTimeGrid parent) {        Parent = parent;    }    public void InstantiateIn(Control container) {        GridViewHeaderTemplateContainer ctr = (GridViewHeaderTemplateContainer)container;        string html = "<div style='width:150px'>";        if (Parent.MayEdit)            html += string.Format("<img alt='{0}' title='{0}' src='Img/add18d.png' style='cursor: pointer' onclick='StatRefTimeGrid.addRow()' />", Texts.CompValue.AddRefTime[Parent.Parent.State.LangIdx]);        if (ctr.Grid.VisibleRowCount > 1)            html += string.Format("<img alt='{0}' title='{0}' src='Img/refresh18d.png' style='cursor: pointer' onclick='StatRefTimeGrid.refresh()' />", Texts.CompValue.RefreshRefTimeGrid[Parent.Parent.State.LangIdx]);        html += "</div>";        ctr.Controls.Add(new LiteralControl(html));    }}

Approach #2

private class AddBtnTemplate : ITemplate {    private readonly StatRefTimeGrid mParent;    public AddBtnTemplate(StatRefTimeGrid parent) {        mParent = parent;    }    public void InstantiateIn(Control container) {        GridViewHeaderTemplateContainer templateContainer = (GridViewHeaderTemplateContainer)container;        HtmlGenericControl div = new HtmlGenericControl("div") {            Style = {[HtmlTextWriterStyle.Width] = "150px"},        };        if (mParent.MayEdit) {            AddNewImage(div, Texts.CompValue.AddRefTime[mParent.Parent.State.LangIdx], "~/Img/add18d.png", "StatRefTimeGrid.addRow()");        }        if (templateContainer.Grid.VisibleRowCount > 1) {            AddNewImage(div, Texts.CompValue.RefreshRefTimeGrid[mParent.Parent.State.LangIdx], "~/Img/refresh18d.png", "StatRefTimeGrid.refresh()");        }        templateContainer.Controls.Add(div);    }    private void AddNewImage(HtmlGenericControl div, string altText, string imageUrl, string onClick) {        div.Controls.Add(new Image {            AlternateText = altText,            ToolTip = altText,            ImageUrl = imageUrl,            Attributes = {                ["onclick"] = onClick,            },            Style = {                [HtmlTextWriterStyle.Cursor] = "pointer",                [HtmlTextWriterStyle.Position] = "relative",                [HtmlTextWriterStyle.Top] = "3px",            },        });    }}

I'm looking for arguments for or against either solution. What do you think?

Mast's user avatar
Mast
13.9k12 gold badges57 silver badges128 bronze badges
askedJul 6, 2018 at 7:20
Kim Homann's user avatar
\$\endgroup\$

2 Answers2

3
\$\begingroup\$

Second approach is better, according to the rule: do not repeat the code!

Don't use '+=' to join string, in your case better to use StringBuilder:

var sb = new StringBuilder();sb.Append();...sb.Appden();...return sb.ToString();

I think this is better way, StringBuilder is made for this.You can refactor this code better little more :-)

answeredJul 6, 2018 at 11:42
DawidP's user avatar
\$\endgroup\$
1
  • \$\begingroup\$Thank you for your opinion. Actually I was looking for arguments for or against the server-side controls vs. plain HTML. I didn't mean to give the string solution a disadvantage by repeating code and using += instead of StringBuilder.\$\endgroup\$CommentedJul 9, 2018 at 10:40
3
\$\begingroup\$

The two main things that a control object gives you is 1) easier access to setting properties and 2) automatic HTML encoding, at the cost of more lines of code (in general). In your case, the HTML is simple enough and there are few enough branches that I'd be ok with the LiteralControl approach.

The one thing to watch out for is that your alt and title strings could easily contain apostrophes (maybe it's not likely in English, but do you localize to French?), and that will break your HTML. I like to keep the string tables clean of formatting, so that means the values have to be encoded before rendering. If you use the control objects, this will be taken care of. If you use the string approach, you have to explicitly call an encoding function. All strings should be run through this function, even if none of the current translations have any problem characters. If there are strings that never need encoding (because they are e.g. widths or positions that depend on the language) put those in a separate string table.

answeredJul 20, 2018 at 23:59
Pierre Menard's user avatar
\$\endgroup\$

You mustlog in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.