Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Errors connected with ownership qualifiers & copy #42

Closed
rermolov opened this issue Jan 30, 2017 · 2 comments
Closed

Errors connected with ownership qualifiers & copy #42

rermolov opened this issue Jan 30, 2017 · 2 comments

Comments

@rermolov
Copy link

rermolov commented Jan 30, 2017

Hi guys!
Thank you for library, but I see couple of problems in it.
First of all, I want to draw your attention to problems that are connected with ownership qualifiers.
I will show you a few of them, but you will get the point.
For example, these properties in OHContact.h:

@property (nonatomic, nullable) NSString *fullName;
@property (nonatomic, nullable) NSOrderedSet<OHContactField *> *contactFields;

Default ownership qualifier for object types in ARC is strong
The problem here is that you can set NSMutableString or NSMutableOrderedSet instead and change them in another place; the values will change in this object too.
It will lead to some hard-to-debug problems.

Second problem is connected with copy method. There is an example from OHContact.m:

@interface OHContact ()

@property (nonatomic, readwrite) NSMutableSet<NSString *> *tags;
@property (nonatomic, readwrite) NSMutableDictionary<NSString *, id> *customProperties;

@end

Again, you defined both properties as strong.

- (id)copyWithZone:(NSZone *)zone
{
    OHContact *copy = [[OHContact alloc] init];
    ...
    copy.tags = [self.tags copy];
    copy.customProperties = [self.customProperties copy];
    return copy;
}

At this point, you copied both tags & customProperties to newly created object.
There will be no problem, if you don't change them, but it will be a problem, if you try to add some values to them, because copy of mutable object returns immutable copy and you will receive NSSet and NSDictionary accordingly. So, you will receive runtime error when try to add new objects.

@adam-zethraeus
Copy link
Contributor

thanks for highlighting this

@maxwellE
Copy link
Contributor

maxwellE commented Feb 6, 2017

I see the issue @rermolov . I will trace through and fix these issues

maxwellE pushed a commit to maxwellE/ohana-ios that referenced this issue Mar 2, 2017
maxwellE added a commit that referenced this issue Mar 3, 2017
@maxwellE maxwellE closed this as completed Mar 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants