Simple application structure. What do you think?

Question

I'm interested in the way I am retaining my properties and the aspects of memory management. This is for a simple application that edits the information of a class that is stored in a table. There are 3 ViewControllers.

• A list view (list all classes)
• a detail view of the selected item (lists properties of selected class)
• an edit view (lists single property
  of selected class and allows it to be edited)

This is how it is structured at present, what do you think?

ListViewController

@property (nonatomic, retain) NSMutableArray *pools;
@property (nonatomic, retain) PoolFacilityEditController *childController;

To add a new class instance to the table you click an add button that runs this method..

//Loads up the editPoolFacility controller to add a new pool

-(void)add {

PoolFacilityEditController *editController = self.childController;

PoolFacility *aPoolFacility = [[PoolFacility alloc] init];

[self.pools addObject:aPoolFacility];
[aPoolFacility release];


editController.thePoolFacility = aPoolFacility;

editController.pools = self.pools;

[self.navigationController pushViewController:editController animated:YES];

}

The next controller is now loaded up and here are its interesting instance variables. Wise or not I have chose to just assign the pool to the new controller rather than retain. I don't want to unnecessarily retain.

detail View

@property (nonatomic, assign) PoolFacility *thePoolFacility; (assigned in the above add method)

@property (nonatomic, assign) NSMutableArray *pools; (also assigned in the add method)

The detail view has a method that does the following..

- (void)viewWillAppear:(BOOL)animated {


//Pass the copy onto the child controller
if (self.childController.thePoolFacility != self.thePoolFacility) {
 self.childController.thePoolFacility = self.thePoolFacility;
}

}

The pool is passed onto the detail edit controller so it knows the pool it is editing.

Now a user clicks on an individual bit of pool information (e.g name) and the detail view controller pops up. It allows the editing of individual properties.

It's interesting properties look like this:

@property (nonatomic, retain) PoolFacility *thePoolFacilityCopy;

@property (nonatomic, assign) PoolFacility *thePoolFacility;

And it creates a copy to edit in case the user changes the values and then wants to cancel. If the user presses save it copies the values from the copy into the non-copy.

- (void)viewWillAppear:(BOOL)animated {

PoolFacility *poolCopy = [self.thePoolFacility copy];
self.thePoolFacilityCopy = poolCopy;
[poolCopy release];

}

If save or cancel is pressed the view is popped.

And then we're back to the middle view that displays all the fields.

Now if the user presses save I just poptheviewcontroller and we're back to the list view. OR if the user presses cancel I run this method.

-(void)cancel {

[self.pools removeObject:self.thePoolFacility];
[self.navigationController popViewControllerAnimated:YES];

}

So to summarize

I am assigning a property throughout different view controllers rather than retaining it.

Also my view controllers are only loaded once and are not deallocated when they 'dissapear'

I hope this made some sense! My question is.. Is this a good way of doing it?

Thanks,

Dan

Answer 1

I didn't see a specific question here, so I'll just make some general critiques.

1. In iPhone OS, Cancel buttons are common on dialogs meant to add a new item, but much less so on edit dialogs. In fact, the only example of a Cancel button on an Edit dialog I can think of is in the Clock app's Alarm panel. So don't worry about copying the PoolFacility and copying the changes back when it's saved; just make the Cancel button only be visible for new objects (or use the Trash icon--canceling a new pool and deleting an existing one are actually the same action the way things are designed right now).

2. As you have things now, there's no danger of an object being deallocated at the wrong time. However, if you ever change the storage method--for example, making the app lazily load PoolFacility objects from the disk--it will come back to bite you. Write it properly today and you'll save yourself pain tomorrow. The proper way is to make the thePoolFacility a retained property and release it in your dealloc method. (If you keep managing the pools list the way you currently do, you should do the same thing with it.)

3. Speaking of which, you don't show how existing PoolFacility objects are loaded. Where do they come from? If there's some kind of database access going on, you may find it helpful to have PoolFacility send notifications when an object is created, updated or deleted, and then observe and react to the appropriate notifications as needed. All of the apps I've written that store user data take this approach; I've found it very handy and flexible.

4. Since there's only one pool list and it's needed by multiple controllers, there's no shame in storing it in your app delegate instead of passing it around. Better yet, write a FacilityList singleton object that manages the list. This could allow you to take a lot of logic out of your controllers. Generally, you should put everything you can into your models, except the stuff that interacts with the screen. That means that when Apple makes the iTablet or releases the Apple TV SDK--or just when you decide to make a Mac version or redo the user interface--you can bring as much of your app as possible over unmodified.

Answer 2

Where is your model?