HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Controls to dump key/value pairs

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
dumpcontrolsvaluepairskey

Problem

Are there alternative (better) ways to factor out the common parts other than my use of abstract base class? I find myself following this pattern lately and it makes sense when I code it but sometimes leads to code that is particularly susceptible to obfuscation as time goes by (i.e. I come back to it later and have a hard time remembering how it worked). I am curious to know if it looks good and readable or if maybe it's over-optimized or has other issues.

public abstract class DumpKeyValuePairs : Control
{
    static string XsltPath = "~/Controls/DumpKeys.xslt";
    protected override void OnLoad(EventArgs e)
    {
        this.Controls.Add(new Xml {
            DocumentContent = new XElement("keys",
                                    from key in this.Keys
                                    select new XElement("key",
                                                new XAttribute("name", key),
                                                this[key])).ToString(),
            TransformSource = DumpKeyValuePairs.XsltPath,
        });
        base.OnLoad(e);
    }
    abstract protected IEnumerable Keys { get; }
    abstract protected string this[string index] { get; }
}

public class DumpServerVariables : DumpKeyValuePairs
{
    protected override IEnumerable Keys
    {
        get { return HttpContext.Current.Request.ServerVariables.AllKeys; }
    }

    protected override string this[string index]
    {
        get { return HttpContext.Current.Request.ServerVariables[index]; }
    }
}

public class DumpSessionVariables : DumpKeyValuePairs
{
    protected override string this[string index]
    {
        get { return HttpContext.Current.Session[index].ToString(); }
    }

    protected override IEnumerable Keys
    {
        get { return HttpContext.Current.Session.Cast(); }
    }
}


XSLT:





















Solution

This bit is a little too much to digest:

this.Controls.Add(new Xml {
    DocumentContent = new XElement("keys",
                            from key in this.Keys
                            select new XElement("key",
                                        new XAttribute("name", key),
                                        this[key])).ToString(),
    TransformSource = DumpKeyValuePairs.XsltPath,
});


The trailing comma is annoying, but the LINQ query syntax is a little bit overboard. The instruction is doing too many things.

Wouldn't this be clearer?

Controls.Add(xml);


There's no need to qualify XsltPath with the DumpKeyValuePairs type, even if the field is static - you're in the same type anyway. Actually, this:

static string XsltPath = "~/Controls/DumpKeys.xslt";


Should really be this:

private static readonly string XsltPath = "~/Controls/DumpKeys.xslt";


Now, in order to turn that LINQ jam into a simple Controls.Add(xml); instruction, things need to happen.

var xml = new Xml 
{ 
    DocumentContent = CreateDocumentContent(),
    TransformSource = XsltPath
};


The CreateDocumentContent() method could be where that LINQ query lives.

private string CreateDocumentContent()
{
    return new XElement("keys", Keys.Select(CreateKeyElement)).ToString();
}


And that CreateKeyElement method group, extracted to its own one-liner method again, makes it pretty clear what's going on:

private XElement CreateKeyElement(string key)
{
    return new XElement("key", new XAttribute("name", key), this[key]);
}


Cramming all these functions into a single instruction is making maintenance much harder than it needs to be.

abstract protected string this[string index] { get; }


Maybe it's just me, but in my mind an index is an int that specifies the position of an item in a collection. What you really have here is a string name for accessing the values in some dictionary; correctly naming the indexer parameter can avoid confusion here.

Code Snippets

this.Controls.Add(new Xml {
    DocumentContent = new XElement("keys",
                            from key in this.Keys
                            select new XElement("key",
                                        new XAttribute("name", key),
                                        this[key])).ToString(),
    TransformSource = DumpKeyValuePairs.XsltPath,
});
Controls.Add(xml);
static string XsltPath = "~/Controls/DumpKeys.xslt";
private static readonly string XsltPath = "~/Controls/DumpKeys.xslt";
var xml = new Xml 
{ 
    DocumentContent = CreateDocumentContent(),
    TransformSource = XsltPath
};

Context

StackExchange Code Review Q#9463, answer score: 3

Revisions (0)

No revisions yet.