AddRange or other method to directly create an output list instead of ForEach-CodePudding

I would like to optimize the code below.

My idea is to skip the foreach and use something like AddRange to directly create the errorList output.

I'm new to this, unfortunately I couldn't make it work since LogError and Products have different properties.

Is it be possible? How can I achieve that?

public class Products
    {
        public int reportLine { get; set; }
        public string code { get; set; }
        public decimal price { get; set; }
        public int otherInfo1 { get; set; }
        public int otherInfo2 { get; set; }
        ...
    }

var listProducts = new list<Products>
    {
    //Data from Excel file
    };

var problems = listProducts.Where(p => p.price == null).ToList(); //Example

if (problems.Count() > 0)
{
   foreach(var aux in problems)
      {
          var error = new LogError()
             {
                 Message = "Price not found",
                 ExcelLine = problem.reportLine,
             };
          errorList.Add(error);
      }
}

CodePudding user response：

Use .Select().

if (problems.Count() > 0)
{
    errorList.AddRange(problems.Select(p
        => new LogError()
           {
               Message = "Price not found",
               ExcelLine = p.reportLine
           }));
}

CodePudding user response：

When you say you want to skip the foreach loop, then I need to point out that in this case it is not possible to avoid iterating through your list. However it is possible to minimise the amount of time you are doing it. In your implementation you are iterating through the products list once to find all products without a prices, to create a new list of products without price. Then you are iterating though the list of products without price to add a log entry in your log list. If you have N products in your original product list, then in worst case you have N products without prices. In other words you might iterating through your products list twice!

To reach the best optimized result, you need to find out what is the minimum amount of times it necessary for your program to iterate through the products list? The answer is obviously only once.

In this case you can simply avoid creating your list for products without price, and go directly to adding your log entry.

var listProducts = new list<Products>
    {
    //Data from Excel file
    };

foreach(var product in listProducts)
{
   // skip if product have price
   if(product.Price != null) continue;
   // add log
   errorList.Add(new LogError
   {
       Message = "Price not found",
       ExcelLine = product.reportLine,
   };
}

CodePudding user response：

To me, the most intuitive way to solve this problem is to write a function that accepts a product and analyzes it for any errors.

IEnumerable<Error> ValidateProduct(Product product)
{
    if (product.Price == null) yield return new Error { Message = "Price not found", ExcelLine = product.reportLine };

    if (product.OtherProperty == null) yield return new Error { Message = "Some other error", ExcelLine = product.reportLine };

    //Etc.... I'm sure there's other validations to do, right?
}

You can then get your error list like this:

var errors = products.SelectMany(ValidateProduct).ToList();

Ta da! This way your business logic is contained in its own function independent of the rest of the code.