3

Ok, so there are a few excepotions here, I cannot use List<Dvd> Dvds = _dvds.ReadAll(); in DvdController.cs and check to see if it contains the dvd info if dvd is already in the list. Even if I did do that, it does not work as I intended it to do. Even if I check to see if the info is in the list and try to stop it, it still adds it to a list. The Dvd.cs does increment the Id by one by the way. I would like to know what would be the solution to this?

DvdController.cs

...
private void CreateDvd() //Create
    {
        var myView = new DvdView();
        var dvdInfos = myView.GetNewDvdInfo();

        _dvds.Create(dvdInfos);
        DisplayDvds();
    }
...

DvdRepository.cs

public class DvdRepository
{
    private static List<Dvd> dvds = new List<Dvd>()
    {
        new Dvd("Batman", 2010, "Bruce", 4 ),
        new Dvd("Superman", 2009, "John", 4),
        new Dvd("Wonderwoman", 2012, "Omar", 4)
    };
    public Dvd Create(Dvd dvd)
    {
        if (dvds.Contains(dvd))
        {
            Console.WriteLine("duplicate"); //not working
        }
        else
            dvds.Add(dvd);

        return dvds.FirstOrDefault(d => d.Id == dvd.Id);
    }
    public List<Dvd> ReadAll()
    {
        return dvds;
    }
...

Dvd.cs

public class Dvd
    {
        public Dvd(string title, int releaseyear, string director, float rating)
        {
            Id = Interlocked.Increment(ref globalId);
            Title = title;
            ReleaseYear = releaseyear;
            Director = director;
            Rating = rating;
        }

        public static int globalId;
        public int Id { get; private set; }
        public string Title { get; set; }
        public int ReleaseYear { get; set; }
        public string Director { get; set; }
        public float Rating { get; set; }
Alejandro H
  • 388
  • 1
  • 9
  • 32
  • 2
    For `Contains` to work on the value instead of by reference you'll need to override the `Equals` and `GetHashCode` methods in `Dvd`. Alternatively you could do `if(dvds.Any(d => d.Id == dvd.Id))` if you just want to check for duplicate Ids. – juharr Jul 19 '19 at 17:56
  • 1
    @juharr you should put that as an answer, with the caveat that, if `Dvd` is a generated class (such as Entity Framework pocos), it will generate it as a `partial` class and he can provide his own `partial` class to override the Equals method. As it's a generic list, it'll also need to implement `IEquatable`. – Moo-Juice Jul 19 '19 at 17:58
  • Not checking for duplicate ids, mostly just title – Alejandro H Jul 19 '19 at 18:05
  • @Moo-Juice It doesn't have to implement `IEquatable`, but if you're going to override `Equals` and `GetHashCode` you might was well implement it since the Default comparer will take advantage of it. – juharr Jul 19 '19 at 18:09
  • Possible duplicate of [C# LINQ find duplicates in List](https://stackoverflow.com/questions/18547354/c-sharp-linq-find-duplicates-in-list) – Corv1nus Jul 19 '19 at 18:17
  • You could also use another datatype than `List`. For example a `Dictionary` where the key (`int`) is the ID of the DVD will automatically ensure you do not have duplicates. – Jeppe Stig Nielsen Jul 19 '19 at 18:33

2 Answers2

6

Your check of if (dvds.Contains(dvd)) is looking for that specific object reference. Unless you pass in the actual object that is in the list already, it won't work.

You need to check a unique identifying property of the Dvd. For this you would want to use the .Any() method.

if (dvds.Any(x => x.Title == dvd.Title))
Matt Rowland
  • 4,575
  • 4
  • 25
  • 34
2

Another solution that requires a bit more code upfront but will likely help in other scenarios in the future is to override the Equals and GetHashCode methods on the Dvd class. By default, objects use a reference comparison to determine equality. By overriding these methods, we can use our own logic to determine if two Dvds are equal.

In the example below, I'm using the fields Title, ReleaseYear, and Director, but you could add others as you see fit. I also implemented IEquatable<Dvd> because it's pretty simple (just requires adding an Equals method that takes an object of type Dvd) and it goes along well with the object.Equals implementation:

public class Dvd : IEquatable<Dvd>
{
    public Dvd(string title, int releaseyear, string director, float rating)
    {
        Id = Interlocked.Increment(ref globalId);
        Title = title;
        ReleaseYear = releaseyear;
        Director = director;
        Rating = rating;
    }

    public static int globalId;
    public int Id { get; private set; }
    public string Title { get; set; }
    public int ReleaseYear { get; set; }
    public string Director { get; set; }
    public float Rating { get; set; }

    public bool Equals(Dvd other)
    {
        return other != null &&
               Title == other.Title &&
               ReleaseYear == other.ReleaseYear &&
               Director == other.Director;
    }

    public override bool Equals(object obj)
    {
        return Equals(obj as Dvd);
    }

    public override int GetHashCode()
    {
        return ((Title?.GetHashCode() ?? 17) * 17 +
                ReleaseYear.GetHashCode()) * 17 +
               (Director?.GetHashCode() ?? 17);
    }
}

With these changes in place, we don't have to worry about remembering which fields we need to compare when evaluating two Dvd objects in a Linq query (and if we want to add more properties for comparison, we just do it in one place rather than searching throughout our code), and we can do things like if (firstDvd.Equals(secondDvd)) { // do something if they're equal } .

Now we can use the Contains method as defined in your original code. For example:

private static void Main()
{
    var repo = new DvdRepository();
    repo.Create(new Dvd("Batman", 2010, "Bruce", 2));
}

Outputs to the console: "duplicate"

Rufus L
  • 36,127
  • 5
  • 30
  • 43
  • I am kind of curious what you are doing in `public override int GetHashCode()`, are you encrypting something? – Alejandro H Jul 29 '19 at 16:52
  • @AlejandroH No, just generating a number for an instance of this class based on the fields used in the `Equals` method. `GetHashCode` should always return the same number for classes that are considered equal (though two instances that return the same hash code are not necessarily equal). See [Object.GetHashCode Method](https://learn.microsoft.com/en-us/dotnet/api/system.object.gethashcode?view=netframework-4.8) for more info. – Rufus L Jul 29 '19 at 16:56