3
\$\begingroup\$

The architecture is showing a call to an external Web Service which is called by me. Then I will expose the data using WCF to be callable from another WCF client. But I really don't like how it's done. Interfaces, contracts, implementation in the BLO and implementations on the DAL.

7 time copy & paste of method which just call another identical method.

P.S: I've changed the name of the methods between the image and the code.. In the code I've called the main duplicate method GetExampleData / GetExampleDataInfo.

enter image description here

namespace .Risk.WCF.Services
{
    public static class LeagueServerSingletonContainer
    {
        private static volatile IWindsorContainer serverInstance;
        private static readonly object serverSyncRoot = new Object();

        public static IWindsorContainer ServerContainer
        {
            get
            {
                if (serverInstance == null)
                {
                    lock (serverSyncRoot)
                    {
                        if (serverInstance == null) // double-check
                        {
                            IWindsorContainer serverContainer = SingletonContainerFactory.ConfigureContainer(
                                "LeagueServerWindsor.config")
                                .Install(new LeagueWcfClientsWindsorInstaller());

                            serverInstance = serverContainer;
                        }
                    }
                }
                return serverInstance;
            }
        }
    }
}


namespace .Example.WCF.Services
{
    public class ExampleService : IWCFService, IExampleService
    {
        public ExampleApiResponse<ExampleDataResponse> GetExampleData(int targetId, DateTime? dateFrom,
            DateTime? dateTo)
        {
            var apiSessionKey = InitSessionKeyInformation();
            return ExampleSingleton.Example()
                .GetExampleDataInfo(apiSessionKey, targetId, dateFrom, dateTo);
        }
    }
}


namespace .Example.WCF.Services.Contracts
{
    public static class ExampleServerContainer
    {
        private static volatile IWindsorContainer serverInstance;
        private static readonly object serverSyncRoot = new Object();

        public static IWindsorContainer ServerContainer
        {
            get
            {
                if (serverInstance == null)
                {
                    lock (serverSyncRoot)
                    {
                        if (serverInstance == null) // double-check
                        {
                            IWindsorContainer serverContainer =
                                new WindsorContainer().Install(new ExampleWcfClientsWindsorInstaller());

                            serverInstance = serverContainer;
                        }
                    }
                }
                return serverInstance;
            }
        }
    }
}


namespace .Example.WCF.Services.Contracts
{
    public class ExampleWcfClientsWindsorInstaller : IWindsorInstaller
    {
        public void Install(IWindsorContainer container, IConfigurationStore store)
        {
            container.Install
                (
                    new WcfClientInstaller<IExampleService>(
                        "ExampleServiceEndpoint",
                        Config.ExampleService.ExampleServiceConnectionPoolCapacity,
                        (int) Config.ExampleService.ExampleServiceAcquireChannelTimeout.TotalMilliseconds,
                        "ExampleServiceWcfClient")
                );
        }
    }
}



namespace .Example.WCF.Services.Contracts
{
    [ServiceContract]
    public interface IExampleService
    {
        [OperationContract]
        [FaultContract(typeof (TrackedFault))]
        ExampleApiResponse<ExampleDataResponse> GetExampleData(int targetId, DateTime? dateFrom,
            DateTime? dateTo);
    }
}



namespace .Example.BLL
{
    public class ExampleBLO : IExampleBLO
    {
        private static readonly Object _apiTokenLockObj = new object();
        private readonly ILog log = LogManager.GetLogger(typeof (ExampleBLO));
        private string ExampleApiSessionToken;
        private Timer ExampleKeepAliveTimer;


        public ExampleApiResponse<ExampleDataResponse> GetExampleDataInfo(int targetId, DateTime? dateFrom,
            DateTime? dateTo)
        {
            return ExampleDAOSingleton.Example()
                .GetExampleDataInfo(ExampleApiSessionToken, targetId, dateFrom, dateTo, _apiSessionKey.ApiUrlv2);
        }

        private void ClearApiToken()
        {
            log.Info("Clearing APIToken.");
            ExampleKeepAliveTimer.Stop();
            lock (_apiTokenLockObj)
            {
                ExampleApiSessionToken = null;
            }
        }
    }
}


namespace .Example.BLL
{
    public class ExampleSingleton
    {
        private static volatile IExampleBLO _ExampleBLO;
        private static readonly object _ExampleBLOSyncRoot = new Object();

        /// <summary>
        ///     Restituisce l'istanza statica dell'oggetto per l'accesso ai dati alle terze parti
        /// </summary>
        public static IExampleBLO Example()
        {
            if (_ExampleBLO == null)
            {
                lock (_ExampleBLOSyncRoot)
                {
                    if (_ExampleBLO == null) // double-check
                        _ExampleBLO = new ExampleBLO();
                }
            }
            return _ExampleBLO;
        }
    }
}




namespace .Example.BLL.Interfaces
{
    public interface IExampleBLO
    {
        ExampleApiResponse<ExampleDataResponse> GetExampleDataInfo(int targetId, DateTime? dateFrom,
            DateTime? dateTo);
    }
}


namespace .Example.DAL
{
    internal class ExampleDAO : AbstractDAO, IExampleDAO
    {
        private readonly ILog log = LogManager.GetLogger(typeof (ExampleDAO));

        public ExampleApiResponse<ExampleDataResponse> GetExampleDataInfo(string authenticationToken,
            int targetId, DateTime? dateFrom, DateTime? dateTo, string apiUrlv2)
        {
            string url;

            if (dateFrom == null && dateTo == null)
            {
                url =
                    HttpUtility.UrlPathEncode(
                        string.Format(
                            @"{0}?method=information::get_earnings_market&token={1}&target_id={2}&extended=true",
                            apiUrlv2, authenticationToken, targetId));
            }
            else
            {
                url =
                    HttpUtility.UrlPathEncode(
                        string.Format(
                            @"{0}?method=information::get_earnings_market&token={1}&target_id={2}&datefrom={3}&dateto={4}&extended=true",
                            apiUrlv2, authenticationToken, targetId, dateFrom.Value.ToString("yyyy-MM-dd HH:mm:ss"),
                            dateTo.Value.ToString("yyyy-MM-dd HH:mm:ss")));
            }

            ExampleApiResponse<ExampleDataResponse> response = CallApiMethod<ExampleDataResponse>(url);

            return response;
        }

        private ExampleApiResponse<T> CallApiMethod<T>(string url) where T : ExampleBaseResponse
        {
            log.DebugFormat("Contacting API with request url : {0}", url);

            ExampleApiResponse<T> response = null;
            var serializer = new XmlSerializer(typeof (ExampleApiResponse<T>));
            try
            {
                using (var client = new WebClient())
                {
                    using (Stream stream = client.OpenRead(url))
                    {
                        client.Encoding = Encoding.UTF8;
                        if (stream != null)
                        {
                            response = (ExampleApiResponse<T>) serializer.Deserialize(stream);
                        }

                        if (response != null && response.Response.ParseResult() != Results.Success)
                        {
                            log.WarnFormat("Error in API response. Request url: {0}", url);
                        }
                    }
                }
            }
            catch (Exception)
            {
                log.WarnFormat("Error in API call with request url: {0}", url);
                throw;
            }
            return response;
        }
    }
}




namespace .Example.DAL
{
    public class ExampleDAOSingleton
    {
        private static volatile IExampleDAO _ExampleDAO;
        private static readonly object _ExampleDAOSyncRoot = new Object();

        /// <summary>
        ///     Restituisce l'istanza statica dell'oggetto per l'accesso ai dati alle terze parti
        /// </summary>
        public static IExampleDAO Example()
        {
            if (_ExampleDAO == null)
            {
                lock (_ExampleDAOSyncRoot)
                {
                    if (_ExampleDAO == null) // double-check
                        _ExampleDAO = new ExampleDAO();
                }
            }
            return _ExampleDAO;
        }
    }
}




namespace .Example.DAL.Interfaces
{
    public interface IExampleDAO
    {
        ExampleApiResponse<ExampleDataResponse> GetExampleDataInfo(string authenticationToken, int targetId,
            DateTime? dateFrom, DateTime? dateTo, string apiUrlv2);
    }
}
\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

You've turned your IoC container into a Service Locator (happens when you pass the IoC container around as a dependency)!

public void Install(IWindsorContainer container, IConfigurationStore store)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^ here!

Advantages

  • The "service locator" can act as a simple run-time linker. This allows code to be added at run-time without re-compiling the application, and in some cases without having to even restart it.
  • Applications can optimize themselves at run-time by selectively adding and removing items from the service locator. For example, an application can detect that it has a better library for reading JPG images available than the default one, and alter the registry accordingly.
  • Large sections of a library or application can be completely separated. The only link between them becomes the registry.

But this comes at a price:

Disadvantages

  • Things placed in the registry are effectively black boxes with regards to the rest of the system. This makes it harder to detect and recover from their errors, and may make the system as a whole less reliable.
  • The registry must be unique, which can make it a bottleneck for concurrent applications.
  • The registry can be a serious security vulnerability, because it allows outsiders to inject code right into an application.
  • The registry hides the class' dependencies, causing run-time errors instead of compile-time errors when dependencies are missing.
  • The registry makes the code more difficult to maintain (opposed to using Dependency injection), because it becomes unclear when you would be introducing a breaking change
  • The registry makes code harder to test, since all tests need to interact with the same global service locator class to set the fake dependencies of a class under test.

(http://en.wikipedia.org/wiki/Service_locator_pattern)

By doing this, you've introduced tight coupling between ExampleService and ExampleSingleton, which has become ambient context - which is a DI/IoC anti-pattern that you should strive to eliminate.

The solution is to inject the dependencies through the class' constructor, ideally as an interface (/abstraction - an abstract class works just as well).

Your architecture relies heavily on static classes and the Singleton [anti-]pattern, which is making your solution essentially untestable, as far as I can tell.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks a lot. I think there is no real need to use Singleton or Service Locator here. How should I refactor it? \$\endgroup\$
    – Revious
    Commented Feb 24, 2014 at 16:57
  • 1
    \$\begingroup\$ @Sam: I'd suggest only passing the things you need. You may also want to look into tutorials for your container, because it does support composition. \$\endgroup\$
    – Magus
    Commented Feb 24, 2014 at 17:00
  • \$\begingroup\$ @Sam Get rid of static stuff, inject dependencies into the classes' constructor, let the IoC container determine the lifestyle of dependencies (if you only need 1 instance of something, configure your container to always provide the same instance)... and read Mark Seemann's book ;) (don't have time to provide an answer with an alternative approach right now, might do it later) \$\endgroup\$ Commented Feb 24, 2014 at 17:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.