5
\$\begingroup\$

I have the following method which determines which cars I need to delete from the DB.

private List<CarDTO> BuildCarsToDelete(IList<CarDTO> newCars, IList<CarDTO> existingCars)
{
    var missingCars = new List<CarDTO>();

    var cars = newCars.Select(c => c.CarId);
    var newCarIds = new HashSet<int>(cars);

    foreach (var car in existingCars)
    {
        //If there are no new cars then it had some and they have been removed
        if (newCars.Count() == 0)
        {
            missingCars.Add(car);
        }
        else
        {
            if (!newCarIds.Contains(car.CarId))
            {
                missingCars.Add(car);
            }
        }
    }

    return missingCars;
}

This works as expecting - but if I want to achieve the same functionality for Customers or Apartments of other DTOs I will be copying a pasting the code but only changing the variable names and the Type of DTO around - is there a nicer way possible using generics which would keep the algorithm and logic as it is but allow me to use on any DTO?

\$\endgroup\$
2
  • 2
    \$\begingroup\$ Not really code review. Have your objects implement GetHashCode and Equals. Or have them implement a base class with ID. \$\endgroup\$
    – paparazzo
    Jul 25, 2017 at 14:48
  • \$\begingroup\$ @Paparazzi - ok makes sense - cheers \$\endgroup\$ Jul 25, 2017 at 14:53

3 Answers 3

7
\$\begingroup\$

There is an easier way to find the cars to delete. You just need to use the Except extension.

var carsToDelete = existingCars.Except(newCars);

In order for this to work you have two choices:

  • you can either implement the IEquatable for each DTO or
  • you can implement a common interface for the Id and a custom IEqualityComparer<IYourInterface> and use it with the Except extension.
\$\endgroup\$
2
\$\begingroup\$

No need to implement IEquatable on every DTO, nor is a common interface required. Use generics and a lambda to access a property of a given T:

public static IEnumerable<T> GetItemsToDelete<T, U>(IEnumerable<T> newT, IEnumerable<T> existingT, Func<T, U> selector) {
    return existingT
        .Select(item => new {
            Id = selector(item),
            Item = item
        })
        .Where(a => !newT.Select(selector).Any(b => EqualityComparer<U>.Default.Equals(a.Id, b)))
        .Select(a => a.Item);
}

Usage:

var newT = new [] {
    new Person { Id = 0 },
    new Person { Id = 1 }
};
var existingT = new [] {
    new Person { Id = 0 },
    new Person { Id = 1 },
    new Person { Id = 2 }
};

foreach (var item in GetItemsToDelete(newT, existingT, person => person.Id)) {
    // do something with item
}

Console.ReadKey();
\$\endgroup\$
1
  • \$\begingroup\$ right, there is a third option ;-) \$\endgroup\$
    – t3chb0t
    Jul 25, 2017 at 16:57
0
\$\begingroup\$

Not sure if fix is code review but use a base class
DTO is what you would pass to your method

public abstract class DTO
{
    public int ID { get; set; }
}
public class carDTO : DTO
{
    public carDTO (int id)
    {
        ID = id;
    }
}

Or have your object implement IEqualityComparer and you can just use Except. You could have the base class implement the IEqualityComparer so you don't have to repeat.

Or you can pass an IEqualityComparer to Except

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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