I want to make a static client to call a wcf because the first time I call the wcf it takes too long, even sometimes it throws time out.
I have the following class:
public class Repository : IRepository
{
protected static ServiceClient Client { get; set; }
public Repository()
{
if (Client == null)
{
Client = new ServiceClient();
}
if (Client.ChannelFactory.State != CommunicationState.Opened
|| Client.ChannelFactory.State != CommunicationState.Created)
{
Client = new ServiceClient();
}
}
public virtual async Task<Response> GetPersonsFromAddress(string address)
{
try
{
Request request = new Request();
request.Address = address;
Response response = await Client.GetPersonsAsync(request);
return response;
}
catch (Exception ex)
{
// ...
// Handle exception
// ...
}
}
}
And this class calls the method:
public class Conector : IConector
{
private IRepository Repository { get; }
public Conector()
{
Repository = new Repository();
}
public virtual async Task<Response> GetPersonsFromAddress(string address)
{
Response response = await Repository.GetPersonsFromAddress(address);
return response;
}
}
The object that calls the wcf service is static so I want to know if this is a good practice.
Also if there are a lot of people at the same time using this method, it will return the correct response for each request.
-
1\$\begingroup\$ Is the variable Cliente and Client meant to be the same? \$\endgroup\$BenKoshy– BenKoshy2018年01月18日 23:00:53 +00:00Commented Jan 18, 2018 at 23:00
-
\$\begingroup\$ @BKSpurgeon Yes! Sorry for that, now it is correct \$\endgroup\$Sxntk– Sxntk2018年01月19日 13:36:46 +00:00Commented Jan 19, 2018 at 13:36
1 Answer 1
I think you should consider inverting control and moving ServiceClient
instantiation outside the repository class. Whether or not ServiceClient
is a singleton (in single-instance sense) or not is not something that repository should manage or care about:
public class Conector : IConector
{
private IRepository Repository { get; }
public Conector(IRepository repository)
{
Repository = repository;
}
...
}
public class Repository : IRepository
{
protected ServiceClient Client { get; }
public Repository(ServiceClient client)
{
Client = client;
}
...
}
-
\$\begingroup\$ Is it ok to have 2 constructors? One for the dependency injection and the other one for a default inialization of the inyected class? Like
public Conector()
in the question andpublic Conector(IRepository repository)
like yours? \$\endgroup\$Sxntk– Sxntk2018年01月19日 13:43:52 +00:00Commented Jan 19, 2018 at 13:43 -
\$\begingroup\$ @Sxntk it depends on what default constructor would actually do. If it is going to create and manage some static instance of
ServiceClient
class then it is hardly an improvement. You would just move the problem from one class to another. \$\endgroup\$Nikita B– Nikita B2018年01月22日 10:08:54 +00:00Commented Jan 22, 2018 at 10:08