CODEWITHSUNDEEP
iphoneobjective-cios
Joris Ooms
Joris Ooms

Reputation: 12048

Improvements on a beginner iOS application

I recently started out with iOS programming (with the Stanford CS193P lectures from iTunes U). I got stuck with the homeworks, that dreaded calculator is a bit too hard for me at the moment, so I started writing little, extremely simple applications to get used to the syntax and data structures in Objective-C.

I wrote a small application which has two buttons.. One 'initializes' an array and fills two UILabels with a) the first object (objectAtIndex:0) and b) the count. I then have a button which allows me to cycle through the array.. showing the next object in there when I click, and of course going back to the very first when I reach the end of the array.

This is easy. I know. But alas, I don't get any feedback like a real Stanford student would, and reading books don't really make me feel okay either. So I would like to ask for some feedback and possible code-improvements, best practices, etc. here.

I created a model with one method, which creates a list of items. I then send retain to it, because I was getting EXC_BAD_ACCESS problems without it; so here my first question: is this okay? :) I didn't think I had to retain it at first, and I still don't exactly understand why it works when I do in fact retain it. Or would it be better to somehow return this and 'catch' it in the controller? I really am not sure what the 'good' way is here.

-(void)populateItemsList
{
    itemsList = [NSArray arrayWithObjects:@"Test", @"Test2", @"test3", nil];
    [itemsList retain];
}

On to the next piece of code in my Controller:

I lazily instantiate my model here, because I learnt how to do this from the Stanford Calculator assignment. I suppose this is okay to do? I understand that this way, the memory for the model doesn't come in use until it's actually required?

- (DisplayerModel *)model
{
    if(!displayerModel) 
    {
        displayerModel = [[DisplayerModel alloc] init];
    }

    return displayerModel;
}

I then have a method to 'create' the array and populate my outlets (two UILabels, as mentioned before). 

- (IBAction)createArray:(UIButton *)sender
{
    [[self model] populateItemsList];
    arrayCount = [[[self model] itemsList] count];
    stepper = 0;
    NSString *firstObject = [[[self model] itemsList] objectAtIndex:stepper];
    countDisplay.text = [NSString stringWithFormat:@"%d", arrayCount];
    display.text = [NSString stringWithFormat:@"%@", firstObject];
}

I'm honestly not sure about this code. It might just be me though, because I'm not really used to writing code in Objective-C; are there any improvements I could make here? I'm especially concerned about the whole stepper idea. I was looking for something like currentIndex or similar, but I couldn't seem to find it in the documentation.. that's why I created the stepper variable to keep track of where I am in the array.

And then finally, the method that makes me cycle through:

   - (IBAction)showNextValue:(UIButton *)sender
{
    if (stepper == arrayCount - 1) {
        stepper = 0;
    }
    else {
        stepper ++;     
    }
    display.text = [NSString stringWithFormat:@"%@", [[[self model] itemsList] objectAtIndex:stepper]];
}

I'm not putting my dealloc or viewDidUnload overrides here because well.. I tested this application with Leaks and it doesn't seem to leak any memory. Are there other ways to test this? Build and analyze doesn't report any problems either. Are there other pitfalls I have to look out for?

Thanks to anyone who's willing to look through my code and provide me with some advice, etc. Still learning!

Upvotes: 3

Views: 185

Answers (2)

bshirley
bshirley

Reputation: 8356

By convention, class methods using the class name at the beginning return an autoreleased object. Whereas, alloc/init or new return an object with a single retain count.

Thus:

foo = [[NSArray array] retain];
foo = [[NSArray alloc] init];
foo = [NSArray new];

are all equivalent and create an object (an empty array) with a retain count of 1

Lazy creation of needed objects is generally a good practice. It's also good to release those things in a variety of situations, viewDidUnload and memory warnings are two common places for that. If you do that, you might want to save the state in some way - often NSUserDefaults.

The stepper variable is perfectly fine. You could encapsulate it within your model if you wanted to. But in the controller is an acceptable location as well.

Upvotes: 3

Yuji
Yuji

Reputation: 34195

Looks pretty good. NSArray doesn't have anything like a currentIndex. An array just contains elements. The same array can be referenced by many objects. If two objects x and y use an array a, a can't keep track of currentIndex for users x and y separately. It's the responsibility of x and y to have its own stepper, as you did.

Upvotes: 1

Related Questions