patterncsharpModerate
Copy autodiscovery results from list to an array
Viewed 0 times
arrayautodiscoveryresultslistfromcopy
Problem
I copy addresses from the endpoints to an array that should only hold the endpoint addresses. This works but it seems so old school :/
Can I make this faster, better, more sexy (LINQ)?
Can I make this faster, better, more sexy (LINQ)?
public static EndpointAddress[] Find()
{
EndpointAddress[] endpointAddresses = null;
var discoveryClient = new DiscoveryClient(new UdpDiscoveryEndpoint());
var findResponse = discoveryClient.Find(
new FindCriteria(
typeof(ICommunicationService)));
if(findResponse != null)
{
if(findResponse.Endpoints.Count > 0)
{
endpointAddresses = new EndpointAddress[findResponse.Endpoints.Count];
for (int i = 0; i <= findResponse.Endpoints.Count; i++)
{
endpointAddresses[i] = findResponse.Endpoints[i].Address;
}
}
}
return endpointAddresses ?? new EndpointAddress[0];
}Solution
You certainly can! You're currently coding using the "arrow pattern" it's often nicer to return early rather than keep indenting further and further as you nest conditionals.
Now let's make the rest of the method shorter:
But, we can go further. You should prefer returning interfaces rather than concrete representations. Do you need indexed access to the EndpointAddresses? I doubt it, return
You could also inject the
if (findResponse == null)
{
return new EndpointAddress[0];
}Now let's make the rest of the method shorter:
public static EndpointAddress[] Find()
{
var discoveryClient = new DiscoveryClient(new UdpDiscoveryEndpoint());
var findResponse = discoveryClient.Find(
new FindCriteria(
typeof(ICommunicationService)));
if(findResponse == null)
{
return new EndpointAddress[0];
}
return findResponse.Endpoints.Select(endpoint => endpoint.Address).ToArray();
}But, we can go further. You should prefer returning interfaces rather than concrete representations. Do you need indexed access to the EndpointAddresses? I doubt it, return
IEnumerable instead:public static IEnumerable Find()
{
var discoveryClient = new DiscoveryClient(new UdpDiscoveryEndpoint());
var findResponse = discoveryClient.Find(
new FindCriteria(
typeof(ICommunicationService)));
if(findResponse == null)
{
return Enumerable.Empty();
}
return findResponse.Endpoints.Select(endpoint => endpoint.Address);
}You could also inject the
DiscoveryClient into the classes constructor but as it is a static method and I don't know what the rest of the class looks like, I'll leave that point for now.Code Snippets
if (findResponse == null)
{
return new EndpointAddress[0];
}public static EndpointAddress[] Find()
{
var discoveryClient = new DiscoveryClient(new UdpDiscoveryEndpoint());
var findResponse = discoveryClient.Find(
new FindCriteria(
typeof(ICommunicationService)));
if(findResponse == null)
{
return new EndpointAddress[0];
}
return findResponse.Endpoints.Select(endpoint => endpoint.Address).ToArray();
}public static IEnumerable<EndpointAddress> Find()
{
var discoveryClient = new DiscoveryClient(new UdpDiscoveryEndpoint());
var findResponse = discoveryClient.Find(
new FindCriteria(
typeof(ICommunicationService)));
if(findResponse == null)
{
return Enumerable.Empty<EndpointAddress>();
}
return findResponse.Endpoints.Select(endpoint => endpoint.Address);
}Context
StackExchange Code Review Q#95070, answer score: 10
Revisions (0)
No revisions yet.