Map client application's DTO to service-proxy's DataContract

Question

My idea is to not not expose/use service's Proxy everywhere throughout the application, rather exercising all service communications(using client Proxy) from a single class. To achieve this I have created below class: -

namespace MyOrganization.EventSender
{
    using MyOrganization.EventSender.DTOs;
    using MyOrganization.EventSender.Proxy;
    using MyOrganization.EventSender.Utilities;
    using log4net;
    using Newtonsoft.Json;
    using System;
    using System.ServiceModel;
    using System.ServiceProcess;

    /// <summary>
    /// The User Session Detail Sender
    /// </summary>
    public class SessionDataSender : ISessionDataSender
    {
        private static readonly ILog log = LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType);
        private SessionDataDTO _sessionDataDTO;

        public SessionDataSender(SessionDataDTO sessionDataDTO)
        {
            _sessionDataDTO = sessionDataDTO;
        }


        public bool Send()
        {
            try
            {
                Proxy.MessageProcessorProxy.SessionDetails sd = new Proxy.MessageProcessorProxy.SessionDetails();
                sd.ChangeTime = _sessionDataDTO.ChangeTime;
                sd.ChangeType = _sessionDataDTO.ChangeType;
                sd.Username = _sessionDataDTO.Username;
                sd.SessionId = _sessionDataDTO.SessionId;               

                log.Info($"Sending SessionEvent [{sd.ChangeType}] to Web-Srver.");

                var client = new Proxy.MessageProcessorProxy.MessageProcessorClient();
                var response = client.LogEventDetails(sd);

                Proxy.ProxyFactory.CloseProxy(client as ICommunicationObject);

                log.Info($"Web-Server Response: [{response.ResponseMessage}]. Reference: [ {JsonConvert.SerializeObject(sd)} ] ");

                return true;
            }
            catch (Exception ex)
            {
                log.Error($"Exception: [{ex.ToString()}]");

                return false;
            }
        }
    }
}

Now, my questions are: -

• Is it good to map every DTOs into Proxy's DataContract? As my this class will grow with more service calls with different DTOs.
• How this class should be architect so that it doesn't violate SRP/Design-patterns?

Answer 1

Is it good to map every DTOs into Proxy's DataContract? As my this class will grow with more service calls with different DTOs.

Yes, mapping to your own domain improves modularity by decoupling the DTO library from your domain.

How this class should be architect so that it doesn't violate SRP/Design-patterns?

1. Use either a custom or existing mapping framework to remove the boiler-plate mapping code from your flow. You could create a Mapper class.
2. Consider using AOP for refactoring boiler-plate try-catch blocks. You could create a LogEntryExitAndErrorAnnotation class.
3. Your reponse object is not required
4. Use a using block to dispose the client
5. Don't use a boolean, when it is always true on success, and hidden when an exception occured.

adapted:

     [LogEntryExitAndErrorAnnotation]
     public void Send()
     {
          var sd = mapper.Map<Proxy.MessageProcessorProxy.SessionDetails>(_sessionDataDTO);               

          // see notes below why to use an adapter
          using (var client = new MessageProcessorClientAdapter(
              new Proxy.MessageProcessorProxy.MessageProcessorClient()))
                  client.LogEventDetails(sd);
     }

Note on (4)

As the OP pointed out: Dispose could hide the root exception.

To avoid this, you can use an adapter. I would use T4 templates to generate these for all my clients.

Suppose your client interface is

public interface IMessageProcessorClient : IDisposable {
    void LogEventDetails(object data); // or typed class
}

You can use the code below to have the best possible exception details. If no error occured during the operation, the error of Dispose is thrown, else an aggregate error is thrown.

public class MessageProcessorClientAdapter : IMessageProcessorClient {
    public IMessageProcessorClient Source {get; private set;}
    public Exception Error {get;private set;}
    public MessageProcessorClientAdapter(IMessageProcessorClient source) {
        this.Source = source; // check for null
    }
    public void LogEventDetails(object data) {
        try {
            this.Source.LogEventDetails(data);
        } catch (Exception error) {
            this.Error = error;
        }
    }
    public void Dispose() {
        try {
            this.Source.Dispose();
        } catch (Exception error) {
            if (this.Error == null) {
                this.Error = error;
            } else {
                this.Error = new AggregateException(this.Error, error);
            }
        } finally {
            if (this.Error != null) {               
                throw this.Error;
            }
        }
    }