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

Refactor for using ChannelFactory

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

Problem

I use VS 2008 and .NET 3.5

I have this code using CustomChannelFactory and Proxy ServiceReference. I would like refactor using lambdas, Action, ...

any suggestions ?

ServiceDependencias.IDependencias svcDependencias = null;

public void Dependencias(List listaFicherosFormXML, string entorno)
{
    CustomChannelFactory customChannelFactory = null;

    if (!string.IsNullOrEmpty(FicheroConfiguracionWCFService)
        && !string.IsNullOrEmpty(EndpointNameWCFService))
    {
        if (!File.Exists(FicheroConfiguracionWCFService)) 
            throw new FileNotFoundException("Fichero de configuración WCF no encontrado", FicheroConfiguracionWCFService);

        customChannelFactory = new CustomChannelFactory(EndpointNameWCFService, FicheroConfiguracionWCFService);

        var iDependencias = customChannelFactory.CreateChannel();          
        svcDependencias = iDependencias;
    }

    if (svcDependencias == null)
    {
        var depClient = new DependenciasClient();
        svcDependencias = depClient;
    }

    GenerarDependenciasDeFormulariosFmb(listaFicherosFormXML, entorno);

    if (svcDependencias is DependenciasClient)
    {
        ((DependenciasClient)svcDependencias).Close();
        return;
    }

    customChannelFactory.Close();
}

Solution

I don't know any Spanish so it was a bit difficult to understand exactly what was going on (I recommend you provide English translations/descriptions of non-English class names and string literals as comments next time).

-
Why do you create a temporary variable that is only use to set another variable on the very next line instead of just setting it directly? (iDependencias & depClient).

-
You close the channel if it's a DependenciasClient but don't clear the reference to it. svcDependencias is visible outside this method and having it hold a reference to a closed connection may be a problem.

-
You don't check to see if svcDependencias is already set before setting it to a new connection. I don't know if you can re-use the connection, but you should at least close the existing connection before you loose the reference to it.

-
I noticed that you only close the channel factory if the channel is not a DependanciasClient. While this will probably work ok if the channel factory never returns a DependanciasClient, I think a null check would be a clearer solution.

if(customChanelFactory != null)
{
    customChanelFactory.Close();
}


-
Instead of using the 'is' operator followed by a cast, you should use the 'as' operator and check for null.

DependenciasClient client = svcDependencias as DependenciasClient;

if (client != null)
{
    client.Close();
}


-
If svcDependencias only exists at the class level for the benefit of GenerarDependenciasDeFormulariosFmb then personally I would prefer to pass it in as an argument rather then use a class field.

Code Snippets

if(customChanelFactory != null)
{
    customChanelFactory.Close();
}
DependenciasClient client = svcDependencias as DependenciasClient;

if (client != null)
{
    client.Close();
}

Context

StackExchange Code Review Q#1854, answer score: 4

Revisions (0)

No revisions yet.