5
\$\begingroup\$

I'm a newbie and I'm new to C# (thus statics and copy-paste) and I'm painfully aware that this code smells. I'm trying to implement a web service that handles a similar pass-through logic with three different data structures that are processed by three corresponding different methods of an external service.

How can I avoid copy-paste here?

public class OutBoundSvcSender {

  public static string sendDataToOutBoundSvc(AbcRecs[] request) {
    StringBuilder result = new StringBuilder();
    try {
      AbcImportResponse response = sendAbcDataToOutBoundSvc(request);
    } catch (Exception ex) {
      result.Append("\nException Message:\n");
      result.Append(ex.Message);
      result.Append("\nStack Trace:\n");
      result.Append(ex.StackTrace);
      return result.ToString();
    }
    return "OK";
  }

  public static string sendDataToOutBoundSvc(BcdRecs[] request) {
    StringBuilder result = new StringBuilder();
    try {
      BcdImportResponse response = sendBcdDataToOutBoundSvc(request);
    } catch (Exception ex) {
      result.Append("\nException Message:\n");
      result.Append(ex.Message);
      result.Append("\nStack Trace:\n");
      result.Append(ex.StackTrace);
      return result.ToString();
    }
    return "OK";
  }

  public static string sendDataToOutBoundSvc(CdeRecs[] request) {
    StringBuilder result = new StringBuilder();
    try {
      CdeImportResponse response = sendCdeDataToOutBoundSvc(request);
    } catch (Exception ex) {
      result.Append("\nException Message:\n");
      result.Append(ex.Message);
      result.Append("\nStack Trace:\n");
      result.Append(ex.StackTrace);
      return result.ToString();
    }
    return "OK";
  }

  private static AbcImportResponse sendAbcDataToOutBoundSvc(AbcRecs[] request) {
    var Recs = new AbcRecord[request.Length];
    OutBoundSvcImportServiceClient client = new OutBoundSvcImportServiceClient();
    string token = client.Login(new AuthorizationRequest {
      Login = ConfigurationManager.AppSettings["OutBoundSvcServiceLogin"],
      Password = ConfigurationManager.AppSettings["OutBoundSvcServicePassword"]
    }).Token;
    for (int i = 0; i < Recs.Length; i++) {
      AbcRecs received = request[i];

      Recs[i] = new AbcRecord {};
      Recs[i].Id                  = received.Id; 
      Recs[i].Model               = received.Model;
      Recs[i].Type                = received.Type;
      Recs[i].Category            = received.Category;
      Recs[i].YearOfManufacture   = received.YearOfMan.ToString();
      Recs[i].Company             = received.CompanyId;
      Recs[i].AdditionalDocs      = received.AddDocs;
    }
    AbcImportRequest abcRequest = new AbcImportRequest {
      AuthorizationToken = token,
      AbcRecords = Recs
    };
    return client.AbcImport(abcRequest);
  }

  private static BcdImportResponse sendBcdDataToOutBoundSvc(BcdRecs[] request) {
    var Recs = new BcdRecord[request.Length];
    OutBoundSvcImportServiceClient client = new OutBoundSvcImportServiceClient();
    string token = client.Login(new AuthorizationRequest {
      Login = ConfigurationManager.AppSettings["OutBoundSvcServiceLogin"],
      Password = ConfigurationManager.AppSettings["OutBoundSvcServicePassword"]
    }).Token;
    for (int i = 0; i < Recs.Length; i++) {
      BcdRecs received = request[i];

      Recs[i] = new BcdRecord {};
      Recs[i].Id                  = received.Id; 
      Recs[i].FirstName           = received.FirstName;
      Recs[i].LastName            = received.LastName;
      Recs[i].Initials            = received.Initials;
      Recs[i].Position            = received.Position;
      Recs[i].Company             = received.Company;
    }
    BcdImportRequest bcdRequest = new BcdImportRequest { 
      AuthorizationToken = token,
      BcdRecords = Recs
    };
    return client.BcdImport(bcdRequest);
  }

  private static CdeImportResponse sendCdeDataToOutBoundSvc(CdeRecs[] request) {
    var Recs = new CdeRecord[request.Length];
    OutBoundSvcImportServiceClient client = new OutBoundSvcImportServiceClient();
    string token = client.Login(new AuthorizationRequest {
      Login = ConfigurationManager.AppSettings["OutBoundSvcServiceLogin"],
      Password = ConfigurationManager.AppSettings["OutBoundSvcServicePassword"]
    }).Token;
    for (int i = 0; i < Recs.Length; i++) {
      CdeRecs received = request[i];

      Recs[i] = new CdeRecord { };
      Recs[i].CdeId                = received.CdeId;
      Recs[i].CdeNumber            = received.CdeNumber;
      Recs[i].CdeCompanyId         = received.CdeCompanyId;
      Recs[i].CdeCompanyName       = received.CdeCompany;
    }
    CdeImportRequest cdeRequest = new CdeImportRequest {
      AuthorizationToken = token,
      CdeRecords = Recs
    };
    return client.CdeImport(cdeRequest);
  }

}
\$\endgroup\$
4
\$\begingroup\$

A few things you can do. To help in the future (in case of Def being added, etc.) you can make some of these functions generic.

The caveat, is that the way you have it set now you would have to make an interface that AbcImportRequest, etc. would implement.

Let's assume all the ImportRequest objects implement the following interface:

public interface IImportRequest<TRecord>
{
    string AuthorizationToken { get; set; }
    TRecord[] Records { get; set; }
}

And we'll assume all the Recs objects implement this interface:

public interface IRecs<TRecord>
{
    TRecord BuildRecord();
}

This eases our code greatly.

Next, we'll adjust our sendDataToOutBoundSvc method (and rename it at the same time, to follow conventions of Pascal Case).

    public static string SendDataToOutboundSvc<TRecs, TImportRequest, TRecord, TResult>(TRecs[] request, Func<OutBoundSvcImportServiceClient, TImportRequest, TResult> buildResultFunc)
        where TImportRequest : IImportRequest<TRecord>, new()
        where TRecs : IRecs<TRecord>
    {
        StringBuilder result = new StringBuilder();
        try
        {
            TResult response = SendDataToOutboundSvcInternal<TRecs, TRecord, TImportRequest, TResult>(request, buildResultFunc);
        }
        catch (Exception ex)
        {
            result.Append("\nException Message:\n");
            result.Append(ex.Message);
            result.Append("\nStack Trace:\n");
            result.Append(ex.StackTrace);
            return result.ToString();
        }

        return "OK";
    }

So what did we do? We replaced all the strongly-typed calls with some generic ones. We also added a Func parameter that allows us to specify a certain buildResultFunc to build our result.

We could extract that Func out, but I'll leave that up to you to decide.

Next, we rewrite the other sendDataToOutBoundSvc as:

    public static TResult SendDataToOutboundSvcInternal<TRecs, TRecord, TImportRequest, TResult>(TRecs[] request, Func<OutBoundSvcImportServiceClient, TImportRequest, TResult> buildResultFunc)
        where TImportRequest : IImportRequest<TRecord>, new()
        where TRecs : IRecs<TRecord>
    {
        var Recs = new TRecord[request.Length];
        OutBoundSvcImportServiceClient client = new OutBoundSvcImportServiceClient();
        string token = client.Login(new AuthorizationRequest
        {
            Login = ConfigurationManager.AppSettings["OutBoundSvcServiceLogin"],
            Password = ConfigurationManager.AppSettings["OutBoundSvcServicePassword"]
        }).Token;
        for (int i = 0; i < Recs.Length; i++)
        {
            TRecs received = request[i];

            Recs[i] = received.BuildRecord();
        }
        TImportRequest tResultRequest = BuildImportRequest<TImportRequest, TRecord>(token, Recs);
        return buildResultFunc(client, tResultRequest);
    }

More generics. Starting to get the point?

Now, you'll notice that I replaced all that explicit record building code with a received.BuildRecord(). That is part of the second interface we implemented, for AbcRecs it looks like:

public AbcRecord BuildRecord()
{
    AbcRecord rec = new AbcRecord { };
    rec.Id = Id;
    rec.Model = Model;
    rec.Type = Type;
    rec.Category = Category;
    rec.YearOfManufacture = YearOfMan.ToString();
    rec.Company = CompanyId;
    rec.AdditionalDocs = AddDocs;
    return rec;
}

While the class is:

public class AbcRecs : IRecs<AbcRecord>
{

Etc.

Then we have a BuildImportRequest method:

public static TImportRequest BuildImportRequest<TImportRequest, TRecord>(string token, TRecord[] recs)
    where TImportRequest : IImportRequest<TRecord>, new()
{
    return new TImportRequest { AuthorizationToken = token, Records = recs };
}

More generics. Again, these reduce code duplication heavily.

And lastly, our Import___Data methods:

public static AbcImportResponse ImportAbcData(OutBoundSvcImportServiceClient client, AbcImportRequest request)
{
    return client.AbcImport(request);
}

public static BcdImportResponse ImportBcdData(OutBoundSvcImportServiceClient client, BcdImportRequest request)
{
    return client.BcdImport(request);
}

public static CdeImportResponse ImportCdeData(OutBoundSvcImportServiceClient client, CdeImportRequest request)
{
    return client.CdeImport(request);
}

So, now do we use all this? As follows:

OutBoundSvcSender.SendDataToOutboundSvc<AbcRecs, AbcImportRequest, AbcRecord, AbcImportResponse>(new AbcRecs[0], OutBoundSvcSender.ImportAbcData);
OutBoundSvcSender.SendDataToOutboundSvc<BcdRecs, BcdImportRequest, BcdRecord, BcdImportResponse>(new BcdRecs[0], OutBoundSvcSender.ImportBcdData);
OutBoundSvcSender.SendDataToOutboundSvc<CdeRecs, CdeImportRequest, CdeRecord, CdeImportResponse>(new CdeRecs[0], OutBoundSvcSender.ImportCdeData);

This makes it much easier to implement other types as well, all you have to do is create the appropriate objects, and an Import___Data method, and you can tell it what to do from there.


You could also consider (that is, if you have the ability to modify the webserver) using Data Contract to dynamically serialize and deserialize the data. The DataContract is very powerful.

\$\endgroup\$
  • \$\begingroup\$ Thanks! I'm only starting to make my way in OOP and generics / delegates / constraints are not so intimate to me. Your approach almost works. The thing is that the AbcImportRequest, BcdImportRequest CdeImportRequest are ServiceReferences. So they are autogenerated, and we need to add inheritance of IImportRequest<TRecord> to them. I don't like doing this but it is non-blocking. What's blocking is the fact that these requests have different data Properties. One has ARecords the other BRecords. But the interface asks for Records. How to workaround this fact? \$\endgroup\$ – prot Oct 9 '15 at 11:31
  • \$\begingroup\$ Solved it. I've just adjusted the variable names in the auto-generated code. I still don't like it. But it works! \$\endgroup\$ – prot Oct 9 '15 at 11:53
  • \$\begingroup\$ Oh, too soon. No it doesn't. You just cannot rename BRecords to ARecords because it changes the format of the outbound format and you need to change the external referenced service. This is no good. \$\endgroup\$ – prot Oct 14 '15 at 6:19
  • \$\begingroup\$ @prot Where did you get the idea to rename them? You need to make a solid class for each record type, and implement a BuildRecord method in each one. All of the code in my example is just that, example code. \$\endgroup\$ – 410_Gone Oct 14 '15 at 12:24
  • \$\begingroup\$ I did make class for each record type, but it is the interface IImportRequest that doesn't fit. It says that each class that implements it needs to have a property TRecord[] Records. Which if you do have different names of the property in the referenced classes (Import___Request) just cannot be inherited. So it doesn't work. \$\endgroup\$ – prot Oct 15 '15 at 4:04

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.