user1359448
user1359448

Reputation: 1683

Avoid duplicated loops with minor differences

I am working on a project, i have noticed a lot of duplicated code, I would like to consolidate the duplicated code into a single method.

This is a sample of the duplicated code:

foreach (var glider in gliders)
{
    List<PriceDataModel_New> bestPrices = PriceService.GetBestPrices(prices, glider.Value.No, string.Empty, string.Empty, string.Empty, 1);
    var priceGroups = bestPrices.GroupBy(p => p.SalesCode);
    var salesCodePrice = priceGroups.ToDictionary(k => k.Key, v => v.First());
    AddEmptyines(fieldMapping, lines);
    var last = lines.Last();

    foreach (var keyValuePair in fieldMapping.Postions)
    {
        int index = keyValuePair.Key;
        var key = keyValuePair.Value.InternalHeading;
        InsertInLines(last, key, index, "CODE_Id", modelNo + "_" + glider.Value.No);
        InsertInLines(last, key, index, "ItemId", glider.Value.No);
        InsertInLines(last, key, index, "CODE_OptionalName", (glider.Value.ComponentType + " " + glider.Value.ProductFamily).ToLower());
        InsertInLines(last, key, index, "Attr_Family name", family);
        InsertInLines(last, key, index, "CODE_IsOptional", "true");
        InsertInLines(last, key, index, "Model", modelNo);
        InsertInLines(last, key, index, "CODE_OptionalInfo", glider.Value.Size.ToLower());

        if (AddToLinePrice(salesCodePrice, keyValuePair.Value.InternalHeading, index, last))
            continue;
    }
}

      //AppendLines(seatPads, prices, lines, fieldMapping, "", modelNo, family, "linking.Value.SimpleMaterial", "");

foreach (var seatPad in seatPads)
{
    List<PriceDataModel_New> bestPrices = PriceService.GetBestPrices(prices, seatPad.Value.No, seatPad.Value.Variant.Substring(0, 3), string.Empty, string.Empty, 1);
    var priceGroups = bestPrices.GroupBy(p => p.SalesCode);
    var salesCodePrice = priceGroups.ToDictionary(k => k.Key, v => v.First());
    AddEmptyines(fieldMapping, lines);
    var last = lines.Last();

    foreach (var keyValuePair in fieldMapping.Postions)
    {
        int index = keyValuePair.Key;
        var key = keyValuePair.Value.InternalHeading;
        InsertInLines(last, key, index, "CODE_Id", modelNo + "_" + seatPad.Value.No);
        InsertInLines(last, key, index, "ItemId", seatPad.Value.No);
        InsertInLines(last, key, index, "CODE_OptionalName", seatPad.Value.ModelNo.ToLower());
        InsertInLines(last, key, index, "Attr_Family name", family);
        InsertInLines(last, key, index, "CODE_IsOptional", "true");
        InsertInLines(last, key, index, "Model", modelNo);
        InsertInLines(last, key, index, "CODE_OptionalInfo", seatPad.Value.UpholsteryFabric.ToLower() + " black");

        if (AddToLinePrice(salesCodePrice, keyValuePair.Value.InternalHeading, index, last))
            continue;
    }
}

      //AppendLines(linkingDevices, prices, lines, fieldMapping, "", modelNo, family, "linking.Value.SimpleMaterial", "");

      foreach (var linking in linkingDevices)
      {
          List<PriceDataModel_New> bestPrices = PriceService.GetBestPrices(prices, linking.Value.No, string.Empty, string.Empty, string.Empty, 1);
          var priceGroups = bestPrices.GroupBy(p => p.SalesCode);
          var salesCodePrice = priceGroups.ToDictionary(k => k.Key, v => v.First());
          AddEmptyines(fieldMapping, lines);
          var last = lines.Last();

          foreach (var keyValuePair in fieldMapping.Postions)
          {
              int index = keyValuePair.Key;
              var key = keyValuePair.Value.InternalHeading;
              InsertInLines(last, key, index, "CODE_Id", modelNo + "_" + linking.Value.No);
              InsertInLines(last, key, index, "ItemId", linking.Value.No);
              InsertInLines(last, key, index, "CODE_OptionalName", linking.Value.ComponentType.ToLower());
              InsertInLines(last, key, index, "Attr_Family name", family);
              InsertInLines(last, key, index, "CODE_IsOptional", "true");
              InsertInLines(last, key, index, "Model", modelNo);
              InsertInLines(last, key, index, "CODE_OptionalInfo", linking.Value.SimpleMaterial);

              if (AddToLinePrice(salesCodePrice, keyValuePair.Value.InternalHeading, index, last))
                  continue;
          }
      }

The foreach loops above only differ on a few lines. I can not figure out how to make this generic. I have tried with Reflection, Func<> and Delegates, any suggestions are very welcome.

Upvotes: 3

Views: 136

Answers (3)

Phil1970
Phil1970

Reputation: 2623

Well, it is hard to suggest best solution as it is very subjective... Func<> could be appropriate is some cases where the difference are minors or you have only a few differences.

Here using a common interface (maybe with some adapter/facade/bridge if you don't want to change original objects), might be more adequate.

Alternatively, you might also create another class that would have desired fields and simply convert you original data to that new format.

Which solution make more sense? Well, it would be up to you to evaluate.

Class adapter pattern

You might also look at other related design patterns...

Upvotes: 0

Sumit Maingi
Sumit Maingi

Reputation: 2263

implement an interface similar to the below for gliders, seatpads and linkingdevices objects:

   public interface IProduct
    {
        string No { get; }
        string CodeName { get; }
        string Family { get; }
        string ModelNo { get; }

        string CodeInfo { get; }

        IDictionary<string, string> FieldMapping { get; }
    }

and then make a generic function like:

private void Generic<T>(IEnumerable<T> products, string modelNo)
        where T: IProduct

Accept whatever is outside of products (like modelNo in input) and put whatever is specific to the product (change the name if 'product' is not quite right here).

Optionally, I would change the Fieldmapping dictionary to flattened out properties as well like so in case that makes sense (I am not sure of the underlying complexities though):

public interface IProduct
    {
        string No { get; }
        string CodeName { get; }
        string Family { get; }
        string ModelNo { get; }

        string SalesCode { get; }
        string CodeInfo { get; }

        IEnumerable<IProductAttribute> Attributes { get; }
    }

    public interface IProductAttribute
    {
        string InternalHeading { get; }
        int Index { get; } //not sure what this is for.
    }

Upvotes: 3

Alex
Alex

Reputation: 906

For starters, have a base class for glider, seatPad and linking child classes.

xtract the body of the inner foreach's and move it into its own method. Parameter passed should be of base class type, so code would work for all children types.

the rest should be strait forward.

Upvotes: 0

Related Questions