patterncsharpModerate
Generating a URL
Viewed 0 times
generatingurlstackoverflow
Problem
I have a function that generates a URL using a stringbuilder:
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?
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:
Why concatenate when there's an
Considering the repeated use of the
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.