patterncsharpModerate
Switching on hardware ports
Viewed 0 times
portsswitchinghardware
Problem
I have the following function for turning on some ports on a piece of hardware:
```
public Boolean SwitchOn(Int32 portNumber)
{
HttpCommandBuilder builder = new HttpCommandBuilder(_ipAddress);
builder.Command = HttpCommandBuilder.CommandType.SetPower;
String result;
if (portNumber == 1)
{
builder.Port1 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p61=1") == -1)
{
return false;
}
else
{
return true;
}
}
else if (portNumber == 2)
{
builder.Port2 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p62=1") == -1)
{
return false;
}
else
{
return true;
}
}
else if (portNumber == 3)
{
builder.Port3 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p63=1") == -1)
{
return false;
}
else
{
return true;
}
}
else if (portNumber == 4)
{
builder.Port4 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p64=1") == -1)
{
return false;
}
else
{
return t
```
public Boolean SwitchOn(Int32 portNumber)
{
HttpCommandBuilder builder = new HttpCommandBuilder(_ipAddress);
builder.Command = HttpCommandBuilder.CommandType.SetPower;
String result;
if (portNumber == 1)
{
builder.Port1 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p61=1") == -1)
{
return false;
}
else
{
return true;
}
}
else if (portNumber == 2)
{
builder.Port2 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p62=1") == -1)
{
return false;
}
else
{
return true;
}
}
else if (portNumber == 3)
{
builder.Port3 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p63=1") == -1)
{
return false;
}
else
{
return true;
}
}
else if (portNumber == 4)
{
builder.Port4 = true;
result = SendCommand(builder.GenerateUrl());
if (String.IsNullOrEmpty(result))
{
return false;
}
else if (result.IndexOf("p64=1") == -1)
{
return false;
}
else
{
return t
Solution
There are already two good answers here, but in addition:
Here's an example with these suggestions put together:
For what it's worth, here are the code metrics from VS2015:
And for the original:
- Consider using the C# type keywords (
boolintead ofBoolean,intinstead ofInt32,stringinstead ofString)
- Consider using
Containsinstead ofIndexOf
- Consider introducing a method to set the port. If you can't modify
HttpCommandBuilder, an extension method is one option.
- Consider using object initializers
Here's an example with these suggestions put together:
public bool SwitchOn2(int portNumber)
{
var builder = new HttpCommandBuilder(_ipAddress)
{
Command = HttpCommandBuilder.CommandType.SetPower
};
builder.SetPort(portNumber);
var result = SendCommand(builder.GenerateUrl());
return !string.IsNullOrEmpty(result) && result.Contains($"p6{portNumber}=1");
}
internal static class HttpCommandBuilderExtensions
{
public static void SetPort(this HttpCommandBuilder builder, int portNumber)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}
switch (portNumber)
{
case 1:
builder.Port1 = true;
break;
case 2:
builder.Port2 = true;
break;
case 3:
builder.Port3 = true;
break;
case 4:
builder.Port4 = true;
break;
default:
throw new ArgumentOutOfRangeException(nameof(portNumber));
}
}
}For what it's worth, here are the code metrics from VS2015:
Member: SwitchOn2(int) : bool
Maintainability Index: 70
Cyclomatic Complexity: 2
Class Coupling: 2
Lines of Code: 5
Member: SetPort(this HttpCommandBuilder, int) : void
Maintainability Index: 62
Cyclomatic Complexity: 6
Class Coupling: 4
Lines of Code: 12
And for the original:
Member: SwitchOn(int) : bool
Maintainability Index: 45
Cyclomatic Complexity: 13
Class Coupling: 2
Lines of Code: 36
Code Snippets
public bool SwitchOn2(int portNumber)
{
var builder = new HttpCommandBuilder(_ipAddress)
{
Command = HttpCommandBuilder.CommandType.SetPower
};
builder.SetPort(portNumber);
var result = SendCommand(builder.GenerateUrl());
return !string.IsNullOrEmpty(result) && result.Contains($"p6{portNumber}=1");
}
internal static class HttpCommandBuilderExtensions
{
public static void SetPort(this HttpCommandBuilder builder, int portNumber)
{
if (builder == null)
{
throw new ArgumentNullException(nameof(builder));
}
switch (portNumber)
{
case 1:
builder.Port1 = true;
break;
case 2:
builder.Port2 = true;
break;
case 3:
builder.Port3 = true;
break;
case 4:
builder.Port4 = true;
break;
default:
throw new ArgumentOutOfRangeException(nameof(portNumber));
}
}
}Context
StackExchange Code Review Q#110538, answer score: 11
Revisions (0)
No revisions yet.