Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code cleanup: property attributes and manual property synthesis #78

Merged
merged 21 commits into from Apr 29, 2018

Conversation

alunbestor
Copy link
Owner

Based off https://github.com/alunbestor/Boxer/tree/64bit/master, which is a clone of MaddTheSane@234a6a7

This PR cleans up a swathe of classes to do the following:

  • remove manual property synthesis (i.e. @synthesize propertyName = _ivarName) except for properties defined in protocols.
  • remove manual ivar declarations that are already declared by properties.
  • move any remaining ivar declarations into the .m and .mm files.

This PR also clarifies a bunch of redundant or inappropriate @property attributes, according to the following rules:

  • remove assign from non-object properties (this is inferred)
  • add nonatomic to properties that are not expected to be thread-safe (almost all of them)
  • add atomic to properties that actually are expected to be thread-safe (mostly around async operations and rendering)
  • remove nullable from weak properties (this is implied)
  • remove copy, weak and strong from computed readonly properties (these attributes are only relevant for stored properties, not computed ones)
  • mark delegate-style properties as weak, or in rare cases unsafe_unretained (where code clearly relies on the property having a value)
  • mark weak IBOutlets as strong instead (this is now Apple's recommended practice)

Some classes with complex sets of ivars (e.g. BXSession and BXEmulator) have not yet been migrated, as they need additional work to expose necessary properties to subclasses/subcomponents. I have no doubt also missed a bunch of property attributes too.

@alunbestor
Copy link
Owner Author

cc @MaddTheSane you've done a lot of cleanup work around property attributes - this integrates your latest changes, and I'd appreciate your eyes on this in case I've inadvertently regressed something.

Copy link
Contributor

@MaddTheSane MaddTheSane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@alunbestor alunbestor merged commit b8204e9 into 64bit/master Apr 29, 2018
@alunbestor alunbestor deleted the 64bit/remove-manual-synthesis branch April 29, 2018 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants