Labels

Milestone

Assignee

3 participants

Inverse relationships are more Core Data like

In the case where you have, for example:

Employee has one Department
Department has many Employees

If you tried to do:[anEmployee setDepartment: aDepartment];
It would fail to set the inverse "many" relationship (i.e add anEmployee to the employees array). I've made a few changes to the PONSO template to correct this behaviour. I've taken steps to ensure that it won't infinite loop!

Fixed a memory leak when the "many" relationship in a one-to-many is Transient

In the case above, if it was configured as such:

Employee has one Department [Non-Transient]
Department has many Employees [Transient]

Employees would be added to an NSArray, and by default, NSArrays retain any objects added to them. This results in an unexpected memory leak (since one would assume that making it transient, like for the "one" side, causes it to be "assigned"). I've fixed this by using a trick when creating the NSMutableArray (or NSMutableSet, see below) - using CFArrayCreateMutable, you can set the "retain" and "release" functions to NULL to prevent it from calling retain and release on added objects. Since this makes use of the Toll-free bridge, it will cause issues with ARC projects.

NSSet-based PONSO Templates

I've also added a sub-directory to the PONSO templates folder which is basically the same as the NSArray ones except it uses NSSet instead of NSArray. This is particularly useful, since along with my changes to inverse relationships, with NSArrays, the following code would result in unexpected behaviour:[anEmployee setDepartment: aDepartment];
[anEmployee setDepartment: aDepartment];
You'd end up with the employees array containing anEmployee twice, which may not be what the developer was expecting. Using the NSSet-based templates gets around this issue since NSSets only accept unique objects.

On 19.8.2011, at 10.22, tyrone-sudeium <reply@reply.github.com> wrote:
Three major changes in this one:
Inverse relationships are more Core Data like
-------------------------
In the case where you have, for example:
Employee has one Department
Department has many Employees
If you tried to do:
`[anEmployee setDepartment: aDepartment];`
It would fail to set the inverse "many" relationship (i.e add `anEmployee` to the `employees` array). I've made a few changes to the PONSO template to correct this behaviour. I've taken steps to ensure that it won't infinite loop!
Fixed a memory leak when the "many" relationship in a one-to-many is Transient
------------------------
In the case above, if it was configured as such:
Employee has one Department [Non-Transient]
Department has many Employees [Transient]
Employees would be added to an `NSArray`, and by default, `NSArray`s retain any objects added to them. This results in an unexpected memory leak (since one would assume that making it transient, like for the "one" side, causes it to be "assigned"). I've fixed this by using a trick when creating the `NSMutableArray` (or `NSMutableSet`, see below) - using `CFArrayCreateMutable`, you can set the "retain" and "release" functions to `NULL` to prevent it from calling retain and release on added objects. Since this makes use of the Toll-free bridge, it _will_ cause issues with ARC projects.
NSSet-based PONSO Templates
----------------------------------
I've also added a sub-directory to the PONSO templates folder which is basically the same as the `NSArray` ones except it uses `NSSet` instead of `NSArray`. This is particularly useful, since along with my changes to inverse relationships, with `NSArray`s, the following code would result in unexpected behaviour:
`[anEmployee setDepartment: aDepartment];
[anEmployee setDepartment: aDepartment];`
You'd end up with the `employees` array containing `anEmployee` twice, which may not be what the developer was expecting. Using the `NSSet`-based templates gets around this issue since `NSSet`s only accept unique objects.
--
Reply to this email directly or view it on GitHub:
https://github.com/rentzsch/mogenerator/pull/68

1.
If you try to run the sample project, there's now a mismatched endforeach in "contributed templates/Nikita Zhuk/ponso/templates/machine.m.motemplate" on line 211. I think you should add <$endif$> on line 190, but since it's your change you should decide on that.

If you run the sample project you'll get a use-after-free error. Stack trace:

Using NSSet is something I wanted to avoid in my own templates because I see some value in having ordered relationships without requiring sorting at fetch. The best data structure here would be an ordered set. It's already introduced in 10.7 (NSOrderedSet) and probably will be in iOS5. However, to keep compatibility with 10.6 and iOS4 I think the best solution would be just to write a class with similar API as NSOrderedSet and use that in PONSO templates (and switch to native implementation in latest OS versions). For now I've used "external" prevention of duplicates in my ModelXXX subclasses, usually either with naive containsObject: check or with some key-based NSDictionary lookup.

However, if you see that NSSet-based templates would be useful, then I would suggest creating your own "contributed templates" subfolder (e.g. "contributed templates/Tyrone Trevorrow/ponso") and put them there, so you'll get proper credit as well on this work. Feel free to reuse and reconfigure my PonsoTest sample project for your own templates.

Also about using the non-retaining CFArray. It basically means that you end up with an array full of dangling pointers when objects get deallocated. I understand that your goal is to break the retain cycles (I've done it myself in ModelXXX subclasses for now), but it opens the possibility of hard-to-track use-after-release bugs. Have you checked out Mike Ash's weak ref library [1] ? How about using MAWeakArray instead?

I've fixed that autorelease issue by removing the call to self.relationship = nil in dealloc for transient relationships. Since it's in the dealloc method, and the object being dealloc'd isn't retaining those variables, it would be a no-op at best to be setting that property to nil (and in this case, thanks to the set inverse "magic", causing a crash at worst).

As for NSArray vs NSSet, for the project I'm working on it was pretty much a requirement that we use NSSet, so I had to put the work in anyway so I figured I'd contribute it back to this project. We have no need for an "ordered" NSSet, and our project needs to work on iOS 4, so I'm happy enough with my solution for our internal project. If down the line you're going to implement your own NSOrderedSet-based templates, it would be somewhat counterproductive for me to set up my own contributed templates folder for an implementation that's going to be obsolete very shortly anyway. Feel free to remove my NSSet templates, but by all means use it for inspiration in your future implementation.

As for dangling pointers, I'm reluctant to include another dependency for this project. Part of the appeal of this implementation is that, for the most part, it is standalone and just requires you to add a script to your project and have the mogenerator engine installed. Having to do that, and have to manage what could potentially become a complex tree of dependencies would seriously reduce the appeal. For the most part, the memory management model of the relationships should be pretty well hidden from the developer - the idea is that the relationships will more or less "manage themselves", and all you have to do is set the properties here and there (similar to how it works in Core Data). Changing or nil'ing out the relationships should automatically cause the inverses to have the references removed (and thus handle the dangling pointer issue).

…NSCoding support
Created a new subfolder in contributed templates to hold my Set-based
templates, added a missing transient check in all machine.m templates,
added yet another variant of the templates which is Set-based with
NSCoding support.

Sorry about my long absence, been completely snowed under at work. I've taken nzhuk's advice and moved my variant of PONSO into its own contributed templates folder. It's still 99% nzhuk's code though, and all the READMEs and code comments still reflect this. I also merged in some changes I made which were necessary for the project we just finished (NSCoding support, mostly). We've also tested (at least, the non-NSCoding variant) in a production setting and they work really well!

EDIT: Sorry meant we've tested the non-NSCoding variant in a production setting!

On 29.11.2011, at 1.00, Jonathan 'Wolf' Rentzsch <reply@reply.github.com> wrote:
Nikita, I'll be pushing out a new version soonish. Merge Tyrone's stuff if you want it in the next release or just let me know it will sit this one out.
---
Reply to this email directly or view it on GitHub:
https://github.com/rentzsch/mogenerator/pull/68#issuecomment-2909008

Tyrone, looking good, but could you reset all the changes made in "contributed templates/Nikita Zhuk" so that the NSArray -> NSSet changes are only in your subdirectory? This way you'll be free to maintain your set of templates independently from my templates.

Also, you might want to adjust the value of MOGENERATOR_TEMPLATES variable in "Tyrone Trevorrow/ponso/sample project/PonsoTest/PonsoTest.xcodeproj" -> Run Script phase so that it points to your templates directory.

Hi Nikita, I'm not sure what you mean by NSArray -> NSSet changes. I never actually modified the templates at the root level (contributed templates/Nikita Zhuk/ponso/templates) to change them to NSSet, I merely duplicated them and created an NSSet variation in a subfolder (contributed templates/Nikita Zhuk/ponso/templates/NSSet), which I subsequently moved into my own folder (contributed templates/Tyrone Trevorrow/ponso/templates/NSSet). The only remaining changes I made that are still in your templates are where relationships automatically update their inverse. Is this the change you want removed?

It might just be me not getting Github's pull requests (I'm still a bit new to this), but if I open the "Diff" tab of this pull request, I can see that there's a modification of e.g. "contributed templates/Nikita Zhuk/ponso/sample project/PonsoTest/Sources/DataModel/_ModelAssistant.h" where NSArray is replaced by NSSet.

Hi Nikita, I've copied over the PonsoTest project from rentzsch/master and it should be better now. I did make changes on my end to the sample project for testing and I inadvertently committed those changes. As shown in the diff, there should no longer be any references to NSSet in your templates folder.

…NSCoding support
Created a new subfolder in contributed templates to hold my Set-based
templates, added a missing transient check in all machine.m templates,
added yet another variant of the templates which is Set-based with
NSCoding support.