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
    \$\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 '17 at 14:48
  • \$\begingroup\$ @Paparazzi - ok makes sense - cheers \$\endgroup\$ – Ctrl_Alt_Defeat Jul 25 '17 at 14:53
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\$
  • \$\begingroup\$ right, there is a third option ;-) \$\endgroup\$ – t3chb0t Jul 25 '17 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, privacy policy and cookie policy

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