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

Generating a URL

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

Problem

I have a function that generates a URL using a stringbuilder:

internal String GenerateUrl()
    {
        StringBuilder urlBuilder = new StringBuilder("http://");
        urlBuilder.Append(IpAddress);
        urlBuilder.Append("/set.cmd?" + "user=" + Username + "+");
        urlBuilder.Append("pass=" + Password + "+");

        switch (Command)
        {
            case CommandType.Undefined:
                throw new ArgumentException("Command type has not been set");

            case CommandType.SetPower:
                urlBuilder.Append("cmd=setpower+");

                for (int i = 0; i < (Ports.Length-1); i++)
                {
                    urlBuilder.Append(BooleanToStringPortRemap(Ports[i], i + 1) + "+");
                }

                urlBuilder.Append(BooleanToStringPortRemap(Ports[Ports.Length - 1], Ports.Length));
                break;

            case CommandType.ReadPower:
                urlBuilder.Append("cmd=getpower");
                break;

            case CommandType.ReadCurrent:
                urlBuilder.Append("cmd=getcurrent");
                break;
        }

        return urlBuilder.ToString();
    }


This scores quite badly for maintainability and cyclomatic complexity. My question is, is there a better way to construct a URL or if not is there a better way to construct one using a Stringbuilder than what I am currently doing?

Solution

This is a mess:

StringBuilder urlBuilder = new StringBuilder("http://");
urlBuilder.Append(IpAddress);
urlBuilder.Append("/set.cmd?" + "user=" + Username + "+");
urlBuilder.Append("pass=" + Password + "+");


Why concatenate when there's an AppendFormat method?

StringBuilder urlBuilder = new StringBuilder(1024);
urlBuilder.AppendFormat("http://{0}/set.cmd?user={1}+pass={2}+", IpAddress, Username, Password);


Considering the repeated use of the key=value pattern, I wonder: why not use a Dictionary, fill that with all of the necessary values, and at the end use string.join to combine them all? Something like this:

var keyValues = new Dictionary();
keyValues.Add("user", UserName);
keyValues.Add("pass", Password);
keyValues.Add("cmd", commandName);

return "?" + string.Join("+", keyValues.Select(x=>string.Format("{0}={1}", x.Key, x.Value)));

Code Snippets

StringBuilder urlBuilder = new StringBuilder("http://");
urlBuilder.Append(IpAddress);
urlBuilder.Append("/set.cmd?" + "user=" + Username + "+");
urlBuilder.Append("pass=" + Password + "+");
StringBuilder urlBuilder = new StringBuilder(1024);
urlBuilder.AppendFormat("http://{0}/set.cmd?user={1}+pass={2}+", IpAddress, Username, Password);
var keyValues = new Dictionary<string, object>();
keyValues.Add("user", UserName);
keyValues.Add("pass", Password);
keyValues.Add("cmd", commandName);

return "?" + string.Join("+", keyValues.Select(x=>string.Format("{0}={1}", x.Key, x.Value)));

Context

StackExchange Code Review Q#110658, answer score: 12

Revisions (0)

No revisions yet.