1
\$\begingroup\$

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?
\$\endgroup\$
2
\$\begingroup\$

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;
            }
        }
    }
\$\endgroup\$
9
  • \$\begingroup\$ Thanks for review & suggestions. \$\endgroup\$ – Anil Kumar May 22 '19 at 15:18
  • \$\begingroup\$ LogEntryExitAndErrorAnnotation - oh, this will produce a lot of garbage log entries ;-) \$\endgroup\$ – t3chb0t May 22 '19 at 16:24
  • \$\begingroup\$ @t3chb0t well, perhaps this is overkill. If the service is hosted in IIS, you could always enable logs here. \$\endgroup\$ – dfhwze May 22 '19 at 16:27
  • \$\begingroup\$ I treat aspect-oriented logging generally as a sign of I don't know what I'm doing so I'd better log everything ;-] \$\endgroup\$ – t3chb0t May 22 '19 at 16:29
  • \$\begingroup\$ @t3chb0t at my current client, we log this extensively to show other teams we 'did' manage to process their request, so the bug 'must' be at their code ;-) \$\endgroup\$ – dfhwze May 22 '19 at 16:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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