List of objects wont compare correctly when using "ObjectList.Contains(foo)"

Question

Book Class from HelperLibrary.Models.Book.cs

public class Book
{
    public string Title;
    public string Author;
    public string ISBN;

    public Book(string title, string author, string iSBN)
    {
        Title = title;
        Author = author;
        ISBN = iSBN;
    }

}

Call

private void SaveChanges_btn_Click(object sender, RoutedEventArgs e)
    {
        List<HelperLibrary.Models.Book> NewUsersBooks = new List<HelperLibrary.Models.Book>();

        foreach (var x in UserBooks_List.Items)
        {

            foreach(HelperLibrary.Models.Book y in App.GlobalBookList)
            {

                if (y.ISBN == x.ToString())
                {
                    NewUsersBooks.Add(y);
                }
            }

        }


        HelperLibrary.Helpers.SQLHelper.AddBookToUser(App.GlobalUserList[UserList_List.SelectedIndex], NewUsersBooks);


}

Sql call from HelperLibrary.SqlHelper.cs

    public static void AddBookToUser(Models.User user, List<Models.Book> NewBooks)

    {
        List<Models.Book> OnlineUsersBooks = new List<Models.Book>();

        OnlineUsersBooks = GetUsersBooks(user);

        Debug.WriteLine("Online Count: " + OnlineUsersBooks.Count.ToString());

        if (OnlineUsersBooks.Count > 0)
        {


                foreach (Models.Book y in NewBooks)
                {

                    if (!(OnlineUsersBooks.Contains(y)))
                    {

                        using (SqlConnection connection = new SqlConnection(connectionString))
                        {
                            SqlCommand command = new SqlCommand("INSERT INTO Bookings VALUES (@UserId, @Title, @Author, @ISBN)", connection);
                            command.Parameters.AddWithValue("@UserId", user.GetUserID);
                            command.Parameters.AddWithValue("@Title", y.Title);
                            command.Parameters.AddWithValue("@Author", y.Author);
                            command.Parameters.AddWithValue("@ISBN", y.ISBN);

                            Debug.WriteLine(command.ToString());

                            command.Connection.Open();
                            command.ExecuteNonQuery();
                        }
                    }



            }

        }
        else
        {
            foreach (Models.Book y in NewBooks)
            {
                using (SqlConnection connection = new SqlConnection(connectionString))
                {
                    SqlCommand command = new SqlCommand("INSERT INTO Bookings VALUES (@UserId, @Title, @Author, @ISBN)", connection);
                    command.Parameters.AddWithValue("@UserId", user.GetUserID);
                    command.Parameters.AddWithValue("@Title", y.Title);
                    command.Parameters.AddWithValue("@Author", y.Author);
                    command.Parameters.AddWithValue("@ISBN", y.ISBN);

                    Debug.WriteLine(command.ToString());

                    command.Connection.Open();
                    command.ExecuteNonQuery();
                }
            }
        }

    }

GetUserBooks method tested and working fine, returning a list of Books. Do i need some sort of extra override to get the

    if (!(OnlineUsersBooks.Contains(y)))

too compare correctly? This is a rather ruff early-stage code, be kind, still have allot of metrics to improve.

You should override the `Equals` and `GetHashCode` method in `Book` class so that `Contains` can work. — user1672994, Apr 26 '18 at 18:19

score 2 · Accepted Answer · answered Apr 26 '18 at 18:36

There are several ways to handle this. One way, as David Grilach wrote in his answer (now deleted, but Rufus L added another answer showing how to do it), is to override the Equals method - but I would not recommend that unless you really know what you are doing. When you override the Equals method it's recommended to also override the GetHashCode method - and it's easy to do it wrong.

Another way is to change the Contains to Find- that would probably be the easiest way for you to do it:

if (OnlineUsersBooks.Find(b=> b.ISBN == y.ISBN)==null)

Using the Find method allows you to use a lambda expression as a predicate, so you don't have to override Equals or GetHashCode at all.

Yet another way is to use linq. It's very powerful and not that hard to learn, and it can help you write a lot less code then you do now.

Here is an untested example of how I would get the books you need to insert to the database with linq:

var booksToAdd = NewBooks
    .Where(nb => !OnlineUsersBooks
        .Any(ob => ob.ISBN == nb.ISBN));

It will return an IEnumerable<Book> that contains all the books in NewBooks that doesn't have an ISBN match in OnlineUsersBooks, without you having to write the loop to get it.
Another benefit of this approach is that it's eliminating the need for the if(OblineUsersBooks.Count>0) - it will work the same with an empty list.

Also, as a side note, you should not use public fields. Instead, use public properties (Bonus reading: Why?):

public class Book
{
    public string Title {get; set;} // Note the {get;set;} here.
    public string Author {get; set;}
    public string ISBN {get; set;}

    public Book(string title, string author, string iSBN)
    {
        Title = title;
        Author = author;
        ISBN = iSBN;
    }

}

I just stumbled upon the "myList.Any(x => x.someProperty == someValue);" method seconds before reading this, but a firm an well written answer this was! Thanks a ton! — Melvin Langvik, Apr 26 '18 at 18:41

score 0 · Answer 2 · answered Apr 26 '18 at 18:37

In order to make Contains return something useful, you need to override the Equals method on your class. Otherwise, the comparisons are done using a reference comparison (which means it will only return true if one of the books in the collection is pointing to the same memory location as the book you're looking for).

The simplest way to do this is probably to use the ISBN, since for books I believe that's supposed to be a unique identifier. But you could also use the other fields for comparison if you wanted to.

Note that when you override Equals, you should also override GetHashCode as well. Here's a simple example:

public class Book
{
    public string Title;
    public string Author;
    public string ISBN;

    public Book(string title, string author, string iSBN)
    {
        Title = title;
        Author = author;
        ISBN = iSBN;
    }

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

    protected bool Equals(Book other)
    {
        return string.Equals(ISBN, other?.ISBN);
    }

    public override int GetHashCode()
    {
        return ISBN?.GetHashCode() ?? 0;
    }
}

Dour High Arch · Answer 3 · 2018-04-26T19:00:05.540

By default, Collection.Contains performs reference comparisons, so a == b is true only if a and b are the same object. Having the same field values is not sufficient.

There are several different ways of solving this, but if you want instances to compare as equal if their field values are equal you want to implement IEquatable:

public class Book : IEquatable<Book>
{
    public readonly string Title;
    public readonly string Author;
    public readonly string ISBN;

    public Book(string title, string author, string iSBN)
    {
        if (string.IsNullOrEmpty(title)) throw new ArgumentNullException(nameof(title));
        if (string.IsNullOrEmpty(author)) throw new ArgumentNullException(nameof(author));
        if (string.IsNullOrEmpty(iSBN)) throw new ArgumentNullException(nameof(iSBN));

        Title = title;
        Author = author;
        ISBN = iSBN;
    }

    public bool Equals(Book other)
    {
        return other != null
            && other.Title == Title && other.Author == Author && other.ISBN == ISBN;
    }

    protected override bool Equals(object other)
    {
        return Equals((Book)other);
    }

    public override int GetHashCode()
    {
        return Title.GetHashCode() ^ Author.GetHashCode() ^ ISBN.GetHashCode();
    }
}

Depending on how you use Book you may want to make its fields into properties, you may want to override == and !=, and may want to change your GetHashCode implementation, but what I have shown is a good start.

List of objects wont compare correctly when using "ObjectList.Contains(foo)"

3 Answers3