3

I have a UIView subclass that manipulates a number of layers. Because the view's layer's sublayers retains the layers strongly, my thought was to make my own references to them weak:

@interface AnchorCell : UICollectionReusableView
@property (weak, nonatomic) CAShapeLayer *link;
@property (weak, nonatomic) CATextLayer *offset;
@end

I can do a certain amount of setup at init time. But I end up writing code like this:

- (void) initLink {
    CAShapeLayer *shape = _link = [CAShapeLayer layer]; // have to put it in local variable to make it last long enough to get to the sublayer
    _link.strokeColor = [[UIColor orangeColor] colorWithAlphaComponent: 1].CGColor;
    _link.fillColor = nil;
    _link.lineWidth = 3;
    _link.masksToBounds = NO;
....
    [self.layer addSublayer: _link];
}

I'm curious if there's a better idiomatic way to do this. What I like about the above, is that it highlights as much as possible, that I'm setting up the link variable, instead of some local shape variable, which I then set to link at the end. What I don't like about it is that you have to add a local variable for no apparent reason that Xcode warns about.

I could move the addSublayer: to the top of the method:

- (void) initLink {
    CAShapeLayer *shape = [CAShapeLayer layer];
    [self.layer addSublayer: shape];
    _link = shape;
    _link.strokeColor = [[UIColor orangeColor] colorWithAlphaComponent: 1].CGColor;
    _link.fillColor = nil;
    _link.lineWidth = 3;
    _link.masksToBounds = NO;
}

But this hides things (to me) as well. It doesn't make it clear that link has been added as a sublayer. Also, sometimes, there are patterns where you have to do a certain amount of setup to the object before you register it elsewhere.

Is there a more elegant way of doing this? Or at least a more idiomatic way given the restrictions of ARC managed ObjectiveC?

Travis Griggs
  • 21,522
  • 19
  • 91
  • 167
  • 1
    Since view are not unloaded anymore since iOS 6 by default, you could just make the property strong. See e.g. http://stackoverflow.com/a/20506127/1187415. – Martin R Mar 19 '14 at 18:25
  • So you're saying, despite Apple docs and storyboard defaults, the real *idiom* to subscribe to now is just use `strong` and then there's no need to search for an idiomatic method to the above? – Travis Griggs Mar 19 '14 at 18:32
  • Connections to storyboard objects are a bit different, because those objects *are* already instantiated. I just can't think of any drawback in using a strong property here. – Martin R Mar 19 '14 at 18:52
  • @MartinR It's a minor point, but the disadvantage in using `strong` here is that if you ever `removeFromSuperlayer` at some later point, the code that does that now needs to concern itself with whether it also needs to `nil` this property, too (and that's none of its business). The goal of the property's memory semantics is to make a claim of ownership (or not) explicitly so the rest of the code elsewhere doesn't need to concern itself about it. – Rob Mar 19 '14 at 19:36
  • @Rob: That's a valid argument. – Martin R Mar 19 '14 at 19:49

3 Answers3

4

I don't think you should be making properties strong just to avoid the use of a local variable (and if the property is weak, you need the local variable).

I think the memory semantics (e.g. weak vs strong vs copy) should reflect the functional object ownership graph, not sleight of hand just to avoid the use of a local variable. I would leave the property as weak, and then do the obvious:

CAShapeLayer *shape = [CAShapeLayer layer]; // have to put it in local variable to make it last long enough to get to the sublayer
shape.strokeColor = [[UIColor orangeColor] colorWithAlphaComponent: 1].CGColor;
shape.fillColor = nil;
shape.lineWidth = 3;
shape.masksToBounds = NO;
....
[self.layer addSublayer: shape];

self.link = shape;

Personally, I don't I think the line that assigned both shape and _link in a single line of code improved its readability, so I prefer to separate those, but do whatever you want.

Furthermore, I generally advise the use of the setter, because I never know if I might have a custom setter or change the memory semantics at some future date (and the code here shouldn't be concerned about that). More than once I've had to refactor code that used the ivar directly because some implementation detail changed at a later date, but I have yet to regret using the accessor methods.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • OK, this is very compelling. One, I really like your point that if it's my ref to it is weak, I should indeed work with a local variable, since I don't "own" it. Since this was an init method, I was using the _reference directly, since that's common. But I like your idea of using the property setter better. In fact... it dons on me that I could (probably will) actually make a custom setLink method which does the addSublayer there. That way, the management of setting the variable, who owns it, where at, etc, is encapsulated from the initialization of the object. – Travis Griggs Mar 20 '14 at 00:10
1

You might want to try code block evaluation, as described in this blog post.

Like:

    _link = ({
        CAShapeLayer *shape = [CAShapeLayer layer];
        shape.strokeColor = [[UIColor orangeColor] colorWithAlphaComponent: 1].CGColor;
        shape.fillColor = nil;
        shape.lineWidth = 3;
        shape.masksToBounds = NO;
        [self.layer addSublayer: shape];
        shape;
    )};
Joshua Kaden
  • 1,210
  • 11
  • 16
  • The only drawbacks I can see to this approach are that it utilizes a nonstandard language extension (is such a thing really even possible with Objective-C?) and that other people reading the code may be confused by the syntax. It's an extension of my own practice of specifically scoping temporary variables (shape in this case) so I think I'll adopt it! – David Berry Mar 19 '14 at 20:22
  • I don't use block evaluation that often, but since GCC and Clang support it, it's not like it's going anywhere for Mac or iOS development. :-) – greymouser Mar 19 '14 at 21:31
0

I don't think you need the temporary variable shape at all - simply do:

_link = [CAShapeLayer layer];

or

self.link = [CAShapeLayer layer];

Mind you, if you never need _link again once it becomes a sublayer then you can do it all with the local variable and skip the @property.

Hope this helps.

nsdebug
  • 364
  • 2
  • 8
  • 2
    Note that `link` is a *weak* property. Therefore it might be deallocated *immediately* after the assignment, even before it has been added as a subview. – Martin R Mar 19 '14 at 18:27
  • Yes, it gets blown away immediately. – Travis Griggs Mar 19 '14 at 18:31
  • I normally make things strong, see Martin's comment above. Or if you don't need access, simply use a local variable only to set it up and add it as a sublayer. – nsdebug Mar 19 '14 at 18:35