I have 2 objects and I am comparing them to see which fields are equal and which are different.

But I am not sure my loop is 100% correct. Does this look ok? If so, what other ways could this be coded to make it 'neater'. e.g. using LINQ

I want to: loop through each DocumentType in both objects, and for each documentType - I want to loop through each DocumentTypeVersion and compare this.

foreach (DocumentType documentType in documentTypes)
{
    foreach (DocumentType importDocumentType in importedDocumentTypes)
    {

        if (documentType.Id == importDocumentType.Id)//ensure its the same documentType we are comparing
        {
            foreach (DocumentTypeVersion version in documentType.Versions)
            {
                foreach (DocumentTypeVersion importedVersion in importDocumentType.Versions)
                {
                    if (version.Id == importedVersion.Id)//ensure its the same documentTypeVersion we are comparing
                    {
                        //check if the versions are equal
                        bool areVersionsEqual = Equals(version, importedVersion);

                    }

                }
            }
        }
    }
}
  • 1
    The code seems strange.. you compare two lists and then you want to detect the differences of elements that have a different id? aren't you more interested in differences of elements of the same id??? – Stephan Dec 17 '14 at 13:57
  • I only want to check if it has the same ID. I will update my code to show how I am now doing this,. – user3437721 Dec 17 '14 at 13:58
  • yeah I guess so, I only want to compare items that exist in both lists. But if anything is NEW in the imported list then I will need to handle this as a difference – user3437721 Dec 17 '14 at 14:01
up vote 13 down vote accepted

An option would be to create two IEqualityComparers; one for DocumentType and one for DocumentTypeVersion. You could then easily call Intersect() and get the correct ones.

public class DocumentTypeIdComparer : IEqualityComparer<DocumentType>
{
    public bool Equals(DocumentType docType1, DocumentType docType2)
    {
        return !(docType1 == null || docType2 == null) && docType1.Id == docType2.Id;           
    }
}

public class DocumentTypeVersionComparer : IEqualityComparer<DocumentTypeVersion>
{
    public bool Equals(DocumentTypeVersion docTypeVersion1, DocumentTypeVersion docTypeVersion2)
    {
        return !(docTypeVersion1 == null || docTypeVersion2 == null) && Object.Equals(docTypeVersion1, docTypeVersion2);           
    }
}

You will then be able to reduce your code to this;

var equalDocumentsById = documentTypes.Intersect(importDocumentTypes, new DocumentTypeIdComparer());
var equalVersions = documentTypes.Versions.Intersect(importDocumentTypes.Versions, new DocumentTypeVersionComparer());
var equalDocuments = equalDocumentsById.Where(doc => doc.Versions.Intersect(equalVersions, new DocumentTypeVersionComparer()).Any());

And then process equalDocuments as you please. If you want the documents which are not equal, simply switch .Count > 0 with .Count == 0.

  • 6
    Is there a reason you're using .Count > 0 instead of .Any()? The later may improve readability. – Andrew Coonce Dec 17 '14 at 17:56
  • Any() is faster too. And for an IEnumerable<T> it is a metod (), not a property. – Heslacher Dec 17 '14 at 20:54
  • @AndrewCoonce no, good point! Thanks. – Dion V. Dec 17 '14 at 21:06

My first approach:

        foreach (var documentType in documentTypes)
        {
            var importedDoc = importedDocumentTypes.FirstOrDefault(doc => doc.Id == documentType.Id);
            if(importedDoc == null) 
                continue; // or error handling for not found


            foreach (var version in documentType.Versions)
            {
                var importedVersion = importedDoc.Versions.FirstOrDefault(ver => ver.Id == version.Id);
                if(importedVersion == null)
                    continue; // or error handling

                bool areVersionsEqual = Equals(version, importedVersion);

            }
        }

The idea is to remove the granularity of nesting in the code, which makes it more readable and understandable. Read more on Programmers.

Don't forget to write tests for stuff like that, and keep in mind that you only do a one-way check (you check if B is contained in A but NOT A is contained in B). This is equal to your code, and I don't know if you want that.

I know this question has been answered, but I feel like this is a perfect case for the SequenceEqual method. Rather than having to handle the 2 equalitycomparers in the same layer you can change your code into one line--a SequenceEqual() call that kicks off another SequenceEqual inside the comparer to test the versions. So in the outer equalitycomparer you could do the comparison of the Id and then call another SequenceEqual on the Versions collections of both.

Modifying Dion's example above to use SequenceEqual, the revamped comparers: UPDATE: Object.Equals is not going to work for you. You need to compare the properties if you want objects that aren't the exact same but have the exact same values for properties.

public class DocumentTypeComparer : IEqualityComparer<DocumentType>
{
    public bool Equals(DocumentType docType1, DocumentType docType2)
    {
        return !(docType1 == null || docType2 == null) 
               && docType1.Id == docType2.Id 
               && docType1.Versions.SequenceEqual(doctype2.Versions, new DocumentTypeVersionComparer());           
    }
}

public class DocumentTypeVersionComparer : IEqualityComparer<DocumentTypeVersion>
{
    public bool Equals(DocumentTypeVersion docTypeVersion1, DocumentTypeVersion docTypeVersion2)
    {
        if (docTypeVersion1.ReferenceEquals(docTypeVersion2)) return true;
        return !(Object.ReferenceEquals(docTypeVersion1, null) || Object.ReferenceEquals(docTypeVersion2, null)) 
               && docTypeVersion1.SomeProperty == docTypeVersion2.SomeProperty 
               && docTypeVersion1.SomeOtherProperty == docTypeVersion2.SomeOtherProperty);
    }           
}

And your one-liner to check whether the collections differ:

var collectionsMatch = documentTypes.SequenceEqual(importedDocumentTypes, new DocumentTypeComparer());

Example one-liner using possible sorting method:

var collectionsMatch = documentTypes.OrderBy(d => d.Id).SequenceEqual(importedDocumentTypes.OrderBy(d => d.Id), new DocumentTypeComparer());

Note that you will have to sort your collections before doing a comparison. I didn't include what to sort on because that's up to your implementation, but I recommend doing the ordering of Versions inside the DocumentTypeComparer, you don't want the DocumentTypeComparer to only work when the collections it's applied to have a sorted Version property. Also when implementing IEqualityComparer you will have to implement GetHashCode (resources below).

Example implementation of GetHashCode from MSDN: http://msdn.microsoft.com/en-us/library/vstudio/bb342073(v=vs.100).aspx

Discussion on best way to implement the method: https://stackoverflow.com/questions/263400/what-is-the-best-algorithm-for-an-overridden-system-object-gethashcode

  • 1
    Nice answer! About Object.Equals; since the OP uses it in his sample, I figured it would either work for him or he would've been smart enough to figure that piece out by himself. Still, valid point as it will probably always return false. – Dion V. Dec 18 '14 at 21:10
  • Thanks! I generally call it out as a convenience to OP if something introduced along with a solution hasn't been mentioned yet, but I hardly consider it a requirement. – moarboilerplate Dec 19 '14 at 17:22

Var

Use the var keyword when defining foreach loop variables.

e.g.

foreach (DocumentType documentType in documentTypes)

should be

foreach (var documentType in documentTypes)

LINQ

Note this is untested as I don't have access to your types

It's possible to do it in LINQ, although I'm unsure I'd recommend it:

var equalVersions = documentTypes
    .SelectMany(documentType=> documentType.Versions.Select(version=> new {Id=documentType.Id, Version= version}))
    .Where(idVersionPair=> importedDocumentTypes.Any(
        importedDocument=>importedDocument.Id == idVersionPair.Id && 
        importedDocument.Versions.Any(importedVersion=> importedVersion.Equals(idVersionPair.Version))))
    .Select(IdVersionPair=> IdVersionPair.Version);

As you can see, it's fairly unreadable compared to some of the other answers, but I leave it here as proof it can be done.

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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