Instagram / IGListKit

A data-driven UICollectionView framework for building fast and flexible lists.
https://instagram.github.io/IGListKit/
MIT License
12.87k stars 1.54k forks source link

How to test object equality? #199

Closed lucabartoletti closed 7 years ago

lucabartoletti commented 7 years ago

This is a question about testing of equality. IGListKit use the equality to decide if a section should update or not. I think that implementing a complex section object it would be easy to miss the check of a property in the equality and so miss updates of the section. Do you have any suggestion on how I could test that the equality is correctly implemented for an object?

rnystrom commented 7 years ago

@lucabartoletti great question. Definitely something that we're trying to firm up on our end. I actually just made a post to our internal group yesterday about equality checking that I'll summarize here:

Here are the basics to writing good -isEqual: and -hash functions. Note this is all ObjC but translates well to Swift. Also, in IGListKit 2.0.0 we changed the equality method to -isEqualToDiffableObject: (avoids NSObject override).

As an example, if I had a User model with the following interface:

@interface User : NSObject
@property NSInteger pk;
@property NSString *name;
@property NSArray *posts;
@end

I would implement its equality methods like so:

@implementation User

- (NSUInteger)hash {
  return self.pk;
}

- (BOOL)isEqual:(id)object {
  if (self == object) return YES;
  if (![object isKindOfClass:[User class]]) return NO;

  User *right = object;
  return self.pk == right.pk 
  && (self.name == right.name || [self.name isEqual:right.name])
  && (self.posts == right.posts || [self.posts isEqualToArray:right.posts]);
}

@end
rnystrom commented 7 years ago

As far as how to test that the equality checks are correct, I'd write unit tests that compare objects and test what -isEqualToDiffableObject: returns. Then you can lock down exactly how your objects will trigger updates.

lucabartoletti commented 7 years ago

Thanks for the detailed reply @rnystrom.

My main concern is that, following your example of the User object, it will be easy to break the comparison adding a new property even if the User object equality implementation is covered by unit tests. I'd like to be able to catch errors after the first implementation of a model object.

Let's assume that I already have tests that checks different scenarios of the User object equality. Than I add a new property to the User object like

@property NSString *address

All the previous test will succeed cause the sample object will be created with the default value. Not having the address property correctly implemented in the equality could lead to hard to debug bugs.

I'm thinking of a solution where I use Reflection to read the list of properties of a class, than have a protocol that will require two methods

protocol ListDiffable : IGListDiffable {
func testableProperties()->[String]
}

So that I'll be force on all the Diffable objects to declare what are the properties i'm testing in the equality.

@implementation User

- (NSUInteger)hash {
  return self.pk;
}

- (BOOL)isEqual:(id)object {
  if (self == object) return YES;
  if (![object isKindOfClass:[User class]]) return NO;

  User *right = object;
  return self.pk == right.pk 
  && (self.name == right.name || [self.name isEqual:right.name])
  && (self.posts == right.posts || [self.posts isEqualToArray:right.posts])
  && (self.address == right.address || [self.posts isEqual:right.address]);
}

- (NSArray<NSString *>*) testableProperties {
return [NSStringFromSelector(@selectore(name)), 
            NSStringFromSelector(@selectore(address)),
            NSStringFromSelector(@selectore(posts))]
}

@end

Than i can test if

Before digging in this possible solution I wanted to ask you I you have suggestions on how to cover this specific scenario.

rnystrom commented 7 years ago

@lucabartoletti you're exactly right, maintenance does become a little tedious. However with that tedious-ness you get a lot of granular control over your object and list behavior.

For more model automation, it might be worth checking out Facebook Remodel which will do a lot of this via codegen.

jessesquires commented 7 years ago

Going to close this as resolved 😄