Ricky
Ricky

Reputation: 3171

Is it safe to pass properties as pointer-like arguments in a method?

I'm creating a convenience method on my database controller class, which is intended to provide two sub-items for another item. This is what it looks like.

+ (void)getLeft:(out MySubItem *)left 
          right:(out MySubItem *)right 
        forItem:(MyItem *)item
{
    ...
    left = aResult;
    right = anotherResult;
}

In all instances where this method would be used, left and right are strong properties.

[Database getLeft:self.leftItem right:self.rightItem forItem:anItem];

Is there an issue with this? The Static Analyzer also complains because the declarations of left and right are never read (dead store), I'd like to be able to fix this...

Upvotes: 1

Views: 223

Answers (3)

bbum
bbum

Reputation: 162722

Update: As @JohnGibb mentions in the comments, a far better pattern is to not use lazy loading at all. Lazy initialization, especially when it is a potentially very expensive, will incur non-determinate behavior that will likely need to be refactored later because it'll cause responsiveness hiccups or any number of other problems.

Far better to separate loading from use of the various items.


Don't use pass-by-reference. And don't use a block (unless you are going to build a generic object graph traversal/visitation pattern where a "visitor" block is actually quite handy -- but that isn't called for here).

Add a leftItem and rightItem method to your MyItem class. That avoids pass-by-reference, preserves encapsulation in that the MyItem class is now responsible for said relationships (including enabling overrides in subclasses), is straightforward and means that callers can easily chose to only get the left or right, as needed.


A more typical OO pattern:

@interface MyItem : SomeClass
@property(readonly) MySubItem *leftItem;
@property(readonly) MySubItem *rightItem;
@end

@implementation MyItem
- (void) loadUpDaStuff
{
   ... 
}

- (MySubItem*) leftItem
{
  if (self.notLoaded) [self loadUpDaStuff];
  return _leftItem;
}  

- (MySubItem*) rightItem
{
  if (self.notLoaded) [self loadUpDaStuff];
  return _rightItem;
}  

Alternatively, if loading is really expensive, then use a load method + completion block.

Upvotes: 1

Abhi Beckert
Abhi Beckert

Reputation: 33389

This is how I'd approach the same problem using blocks. The syntax is a bit uglier, but I also generally try to avoid pointers.

+ (void)valuesForItem(MyItem *)item completion:(void (^)(MySubItem *left, MySubItem *right))completion
{
  ...

  completion(l, r);
}

[Database valuesForItem:anItem completion:^void(MySubItem *left, MySubItem *right){
  self.leftItem = left;
  self.rightItem = right;
}];

Upvotes: 3

KudoCC
KudoCC

Reputation: 6952

What you want is to change the value of self.leftItem, so you should use address of self.leftItem (it is MySubItem*) as parameter, so you should use MySubItem **.

Try it:

+ (void)getLeft:(out MySubItem **)p_left 
          right:(out MySubItem **)p_right 
        forItem:(MyItem *)item
{
    ...
    *p_left = aResult;
    *p_right = anotherResult;
}

and

MySubItem *l = nil ;
MySubItem *r = nil ;
[Database getLeft:&l right:&r forItem:anItem];
self.leftItem = l ;
self.rightItem = r ;

Updated: I found a solution which don't need two step... I think self.leftItem and self.rightItem is strong property, if not, use __weak instead.

+ (void)getLeft:(out NSObject * __strong *)p_left
          right:(out NSObject * __strong *)p_right
        forItem:(NSObject *)item
{
    *p_left = nil;
    *p_right = nil;
}

Can't use self.leftItem, use _leftItem instead.

[Database getLeft:&_leftItem right:&_rightItem forItem:anItem];

Upvotes: 0

Related Questions