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

Copy autodiscovery results from list to an array

Submitted by: @import:stackexchange-codereview··
0
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)?

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.

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.