views:

250

answers:

4

Hi.

I am new to iphone development. I am "old School" - I was tought to use Procedures etc. when programming. Everything is object oriented these days but my style is still as its always been. Please bear that in mind. My project is tiny and really just a proof of concept.

The program below works on a timer - every 30 seconds it will read a random baby name from my list of names database stored in Dictionary.plist by my app. It then shows this name on the iphone screen.

You can see the full relevant parts of the code below. What happens is - if I increase the timer to run it very fast - eventually it seems to run out of memory or something as it just shows ???? instead of the next random baby name. I suspect this is due to me not closing the database file each time I read it.

Anyway, can someone please look at my code (with my above comments into consideration) and tell me what I need to add to stop it showing ???? after so many runs..

I am only opening the file each time in ShowNextName as I couldnt figure out another way to do it..

I know its not great style to use variables at the start of this code that are global etc. but is there maybe a way to restructure or add something to stop it "crashing" or going a bit funny after so many runs...

I'd appreciated that. Thanks.

#import "BabyNameViewController.h"

@implementation BabyNameViewController


NSDictionary *dictionary;
NSString *name;

int nameCount = 0;
int RecordIndex = 0;


- (void)ShowNextname;
{
    NSString *path = [[NSBundle mainBundle] bundlePath];
    NSString *finalPath = [path stringByAppendingPathComponent:@"Dictionary.plist"];
    NSArray* plistArray = [NSArray arrayWithContentsOfFile:finalPath];

    // Generate a random number between 0 and (the number of records-1) - used as a random index!

    RecordIndex=arc4random()%[plistArray count];


    // Select and display currently selected record from the array.
    dictionary = [plistArray objectAtIndex:RecordIndex];
    name = [dictionary objectForKey:@"name"];

    [nameLabelOutlet setText: [NSString stringWithFormat: @"Random Baby Name is: %@", name]];

}   

// Implement viewDidLoad to do additional setup after loading the view, typically from a nib.
- (void)viewDidLoad {
    [super viewDidLoad];

// Initial App entry point - startup code..


// Open the dictionary to count the number of names and store it for later use.
NSString *path = [[NSBundle mainBundle] bundlePath];
NSString *finalPath = [path stringByAppendingPathComponent:@"Dictionary.plist"];
NSArray* plistArray = [NSArray arrayWithContentsOfFile:finalPath];
nameCount = [plistArray count];

// Generate random name from database   
[self ShowNextname];

// Start up the nameUpdate timer.
[NSTimer scheduledTimerWithTimeInterval:30 target:self selector:@selector(nameUpdate) userInfo:nil repeats:YES];

}

-(void) nameUpdate {
    [self ShowNextname];
}
+1  A: 

The problem may be that every 30 seconds you are reloading the plist into a new array. What you should do is in viewDidLoad load the plist into an instance variable in the object (your view controller class) then just use that instance variable in showNextName. (Also, I'd always name methods starting with a lower case letter and classes with an upper case letter or people might think that ShowNextname is a class instead of a method.)

Also don't forget to add [plistArray release]; in your dealloc method.

Here's a good free document you might want to read that might help some:

http://developer.apple.com/iphone/library/documentation/Cocoa/Conceptual/OOP_ObjC/Introduction/Introduction.html

Nimrod
Thanks - reading that doc now. Yes that's the problem - I am reloading the plist into a new array every time. I know what you are saying but don't know the syntax to implement it - can you maybe show me the line of code to use that loads the plist into an instance variable in the view controller - where I would put it so it is accessible within the showNextName?I will change my code style regading your comments on classes/methods and am deallocating at the end. Thanks for your help.
Steven McDonald
Declare `plistArray` in your .h between the braces. That will make it an instance variable that you can access from any of the `-` methods in your class.
Frank Schmitt
Like Frank says, but you should really do a [plistArray retain] when you set the instance variable to make sure it doesn't get free()d. (The methods `retain` and `release` increment or decrement a reference count which causes free() to be called when the count drops to 0, so since you have a reference you want to increment the count.) Also see http://developer.apple.com/mac/library/documentation/cocoa/Conceptual/ObjectiveC/Articles/ocProperties.html which will tell you how to declare a property that with the `retain` keyword will help maintain the reference count for you.
Nimrod
+1  A: 

You have an extra layer of indirection: you can just do:

[NSTimer scheduledTimerWithTimeInterval:30 target:self selector:@selector(ShowNextName) userInfo:nil repeats:YES];

and eliminate the nameUpdate method.

Frank Schmitt
+1  A: 

A word of advice on transitioning to objects - it helps to think about objects like structs, that you can also have methods on. Instance variables are like items in a struct, where you need a particular struct (the class instance) to get at the exact right value - self is just a way for a method to look at the value held in its own class. So methods in a view controller are all looking at one class instance, the view controller that was created to show your view.

Also the way to think about memory is this, that when you alloc an object that is really just like malloc - init of any flavor is like setting the initial values in a struct (since the malloced memory can be anything initially). Then after that release is what deallocates the memory allocated, but ONLY if there have not been any retain calls. Retain marks that alloced blocvk of memory as used with a counter, and release decrements that counter - when it goes to 0 the memory is gone. That's a simplified view but may help you to start.

Kendall Helmstetter Gelner
Thanks Kendall!
Steven McDonald
A: 

If your app continually uses more memory, run it under Instruments (Leaks template) and see where all that memory is going. Then, drill down into the class whose total size is growing, and look at where the allocation events are happening. Then, you'll be able to find where you're violating the memory management rules.

I can't see any such violation in the code you showed, so the problem must lie elsewhere. Instruments will help you find it.

This video may help you.

Peter Hosey
Thanks Peter That was very helpfull.
Steven McDonald