Adding temporary custom objects to NSMutableArray Problems

Discussion in 'Mac Programming' started by isthisonetaken, Nov 23, 2010.

  1. macrumors regular

    #1
    Hey All,

    so I'm working on a little app, and for right now, to get the interface working correctly, I am adding some static objects to an NSMutableArray, but when that finishes, it adds the right number of objects, but they are all the same as the last object added.

    Item is a custom object with this init method:
    Code:
    - (Item *)init {
    	self = [super init];
    	
    	if (self) {
    		category = @"";
    		storeName = @"";
    		storeAddress = @"";
    		storeHours = @"";
    		
    		name = @"";
    		quantity = 0;
    		price = 0.0;
    		description = @"";
    		picture = @"";
    		upc = 0;
    	}
    	
    	return self;
    }
    Then, in a method in my AppDelegate, I build some tempItems:
    Code:
    Item *tempItem = [[Item alloc] init];
    	// make a new item
    	tempItem.category = @"Groceries";
            // all the other properties are set
    
            [list addObject:tempItem];
            // same code for adding 3 more objects
    
    If I have calls to NSLog before adding objects, it shows that tempObject is what I want it to be. When that is all done, I can see that list has 4 objects in it, but they are all the same as the last object added.

    I feel like this is probably very easy to fix, but searching on Google, I couldn't find anything that worked. I tried releasing, and autoreleasing the tempObject after adding to the list and before changing it to the new object without success.

    Any suggestions?

    Dan
     
  2. Moderator emeritus

    kainjow

    #2
    Post your complete code. If you're not alloc/initing your object after you add it to the array then you're simply modifying the same object over and over.
     
  3. macrumors newbie

    lostp

    #3
    NSMutableArray

    I think he is setting the same objects over and over again. That means all the elements of the NSMutableArray *list point to the same memory location.

    Try this code. Create new function to add to the list.

    Code:
    -(void) addNewItem: (NSString *)categ
    {
         Item *tempName = [[Item alloc] init];
         //assuming category is a property defined as NSString 
         tempName.category = categ;
    
         [list addObject:tempName];
    }
    in your AppDelegate.m just call -addNewItem on every object.

    I hope this helps.
     
  4. macrumors G5

    gnasher729

    #4
    1. As you say yourself, the problem is not in the code that you posted. Post the code that contains the problem.

    2. Turn compiler warnings on. Lots of problems will result in warnings from the compiler.
     
  5. macrumors 603

    whooleytoo

    #5
    Note - you should probably release tempName in the method above; once you've added it to the array. Otherwise that's likely to leak.
     
  6. macrumors 603

    whooleytoo

    #6
    The most likely cause could be you're doing something like this:

    Code:
    Item* tempItem = [[Item alloc] init];
    
    tempItem.category = @"Groceries";
    
    [list addObject: tempItem];
    
    tempItem.category = @"Bananas";
    
    [list addObject: tempItem];
    .
    .
    .
    
    If you do that, you'll end up with 4 duplicate items in the array; since you're changing the original object each time.
     
  7. macrumors newbie

    lostp

    #7
    Oh yea :eek::D also a [tempName release]; Thanks for correcting whooleytoo.
     
  8. macrumors regular

    #8
    Code where I'm adding objects:
    Code:
    Item *tempItem = [[Item alloc] init];
    	// make a new item
    	tempItem.category = @"Groceries";
    	tempItem.storeName = @"Loblaws";
    	tempItem.storeAddress = @"1200 Richmond St East";
    	tempItem.storeHours = @"9am-9pm";
    	tempItem.name = @"milk";
    	tempItem.quantity = 2;
    	tempItem.price = 3.49;
    	tempItem.description = @"2% milk in blue container";
    	tempItem.picture = NULL;
    	tempItem.upc = 123456;
    	// add it to the list array
    	//NSLog(@"tempItem 1 = %@\n", tempItem);
    	[tempItem autorelease];
    	[list addObject:tempItem];
    //	NSLog(@"list count = %d", [list count]);
    	// make a new item
    	tempItem.category = @"Groceries";
    	tempItem.storeName = @"Loblaws";
    	tempItem.storeAddress = @"1200 Richmond St East";
    	tempItem.storeHours = @"9am-9pm";
    	tempItem.name = @"bread";
    	tempItem.quantity = 1;
    	tempItem.price = 1.99;
    	tempItem.description = @"Whole Wheat bread, whatever is on sale.";
    	tempItem.picture = NULL;
    	tempItem.upc = 132657;
    	// add it to the list array
    	//	NSLog(@"tempItem 2 = %@\n", tempItem);
    
    	[list addObject:tempItem];
    //	NSLog(@"list count = %d", [list count]);
    
    	// make a new item
    	tempItem.category = @"Groceries";
    	tempItem.storeName = @"Metro";
    	tempItem.storeAddress = @"800 Oxford St West";
    	tempItem.storeHours = @"24 Hours";
    	tempItem.name = @"chicken";
    	tempItem.quantity = 1;
    	tempItem.price = 12.99;
    	tempItem.description = @"Package of boneless, skinless chicken breasts.";
    	tempItem.picture = NULL;
    	tempItem.upc = 198712;
    	// add it to the list array
    	//	NSLog(@"tempItem 3 = %@\n", tempItem);
    
    	[list addObject:tempItem];
    //	NSLog(@"list count = %d", [list count]);
    
    	// make a new item
    	tempItem.category = @"Electronics";
    	tempItem.storeName = @"Best Buy";
    	tempItem.storeAddress = @"899 Wellington St N";
    	tempItem.storeHours = @"9am-9pm";
    	tempItem.name = @"iPad";
    	tempItem.quantity = 1;
    	tempItem.price = 549.00;
    	tempItem.description = @"16 GB wifi iPad.";
    	tempItem.picture = NULL;
    	tempItem.upc = 817391;
    	// add it to the list array
    	//	NSLog(@"tempItem 4 = %@\n", tempItem);
    
    	[list addObject:tempItem];
    
    code for Item.m
    Code:
    #import "Item.h"
    
    
    @implementation Item
    
    @synthesize category;
    @synthesize storeName;
    @synthesize storeAddress;
    @synthesize storeHours;
    
    @synthesize name;
    @synthesize quantity;
    @synthesize price;
    @synthesize description;
    @synthesize picture;
    @synthesize upc;
    
    - (Item *)init {
    	self = [super init];
    	
    	if (self) {
    		category = @"";
    		storeName = @"";
    		storeAddress = @"";
    		storeHours = @"";
    		
    		name = @"";
    		quantity = 0;
    		price = 0.0;
    		description = @"";
    		picture = @"";
    		upc = 0;
    	}
    	
    	return self;
    }
    
    @end
    I tried putting [tempItem release]; after adding it to the list array, but that didn't help.
     
  9. Moderator

    robbieduncan

    Staff Member

    #9
    As everyone suspected you are using the same item over and over. At the second point where it says "// make a new item" you have not made a new item at all. tempItem is still pointing at the item you created with alloc/init. You need to alloc/init each and every new item.
     
  10. macrumors newbie

    lostp

    #10

    Yes you're adding the same object over and over again.

    first call
    Code:
    [tempItem release]; 
    
    then call
    Code:
    tempItem = [[Item alloc] init]; 
    
    to "create a new item". :). Also read the comments posted above in CAPITAL letters. Sorry I wasn't shouting, I was just making a visual cue on which of them were my comments.
     
  11. macrumors regular

    #11
    Ok guys thanks for the help, I've got it working now. And lostp, the autorelease was just something that was left in from trying things last night. Also, those comments about making a new item just meant changing the attributes, I didn't know I needed to alloc/ init each time!

    Thanks again guys,

    Dan
     
  12. macrumors regular

    #12
    Ok, now I'm having another issue. On my interface, I have a popup button which I want to fill with Item.name and I have a textview which I want to fill with the category, store and item name for each item. The app itself has a NSMutableArray holding Items called list. I wrote two methods:

    Code:
    - (void)updateItemsPopup:(NSMutableArray *)items {
    	// empty the popup than fill it with the names from items
    	[itemListPopup removeAllItems];
    	for (int i = 0; i < [items count]; i++) {
    		[itemListPopup addItemWithTitle:[[items objectAtIndex:i] name]];
    	}
    	
    	[self loadItemDetails:[items objectAtIndex:0]];
    }
    Code:
    - (void)updateAllItemsView:(NSMutableArray *)items {
    	NSString *content = @"";
    
    	// build a string to go in the view
    	for (int i = 0; i < [items count]; i++) {
    		Item *tempItem = [[Item alloc] init];
    		tempItem = [items objectAtIndex:i];
    		content = [content stringByAppendingFormat:@"Category: %@ - Store: %@ - Item: %@\n", [tempItem category], [tempItem storeName], [tempItem name]];
    		[tempItem release];
    	}
    
    	[allItemsView setString:content];
    }
    Then, after I build the Item objects and put them into list, I call those two methods:
    Code:
    [self updateAllItemsView:list];
    		[self updateItemsPopup:list];
    My problem is EXC_BAD_ACCESS. From putting in some breakpoints, it seems like by passing list to the first method, list gets deleted? If I only call one of those methods, then it will run fine (although, after checking, list is still being deleted).

    I tried making a copy of list with:
    Code:
    NSMutableArray *listCopy = [[NSMutableArray alloc] initWithArray:list];
    		[self updateAllItemsView:listCopy];
    		[listCopy release];
    		listCopy = [[NSMutableArray alloc] initWithArray:list];
    		[self updateItemsPopup:listCopy];
    		[listCopy release];
    but that also seems to be deleting list.

    I have declared list in my AppDelagate.h as:
    Code:
    @interface ShoppingAppDelegate : NSObject <NSApplicationDelegate> {
    NSMutableArray *list;
    }
    @property (assign) NSMutableArray *list;
    and synthesized it in the .m and initialize it:
    Code:
    - (void)applicationDidFinishLaunching:(NSNotification *)aNotification {
    	// Insert code here to initialize your application 
    	// initialize the list array
    	list = [[NSMutableArray alloc] initWithCapacity:5];
    }
    I don't see why it is getting deleted whenever I try to do something other than add an object or remove an object?

    Dan
     
  13. macrumors 68040

    lee1210

    #13
    I haven't run the code, but from a quick perusal it's clear that you need to read (or re-read) the memory management guide. You're leaking and over-releasing all over the place.

    -Lee

    Edit: what do you mean by "delete"? The list is empty? The pointer is nil? All of the items are invalid?
     
  14. macrumors regular

    #14
    By deleting, I mean when I go in the debugger, before either of the method calls, I can see list, after one of the method calls, its gone. I know I need to read the memory management guide, I'm assuming I can find that in the developer site? I have garbage collection turned on, the one cocoa book I followed never went over memory management because it always had you turn on the garbage collection.

    Where do you see the leaks? I'm guessing you mean where I tried making a copy of list? If so, I don't have that any more, I was just trying it out.

    Dan
     
  15. macrumors 603

    whooleytoo

    #15
    A few problems there! ;)

    Look at the following code in updateAllItemsView:

    Code:
    Item *tempItem = [[Item alloc] init];
    tempItem = [items objectAtIndex:i];
    content = [content stringByAppendingFormat:@"Category: %@ - Store: %@ - Item: %@\n", [tempItem category], [tempItem storeName], [tempItem name]];		
    [tempItem release];
    
    - You create a new Item, pointed to by tempItem. This is fine.
    - You then point tempItem at a different item from the items array. This is bad. The Item you created in the first line has just been leaked!
    - Finally, you release the item tempItem points to. Since tempItem points to an Item in the items array, this line is now actually releasing the item in the array!

    The following would be better:

    Code:
    Item *tempItem = [items objectAtIndex:i];
    content = [content stringByAppendingFormat:@"Category: %@ - Store: %@ - Item: %@\n", [tempItem category], [tempItem storeName], [tempItem name]];		
    
    - You don't need to 'alloc' a a new Item. The Item already exists; it's in the items array.
    - Since you don't alloc it, you don't need to release it either. Easy peasy.
     
  16. lee1210, Nov 24, 2010
    Last edited: Nov 24, 2010

    macrumors 68040

    lee1210

    #16
    I still am not sure what you mean by "it's gone" here. When you print with the debugger the pointer is 0/nil? The length is 0? Something else? The variable itself can't disappear.

    Also, if you have GC on none of the retain/release stuff should matter. As such, this shouldn't be the problem. With that said, I still think memory management is important to understand.

    -Lee

    Edit:
    http://www.google.com/m/url?client=...UQFjAB&usg=AFQjCNHx2I3KMjRFfDvzc0CHTxDRchKZrA
    There's the memory management guide.

    Whooleytoo pointed out where you were leaking and over-releasing, but with GC that should not be an issue.
     
  17. macrumors regular

    #17
    :( Well now I feel dumb. Turns out when I said I had garbage collection enabled, it was enabled on an older version of this project. Its on now though. Also, before turning it on, whooleytoo's suggestions did the trick!

    As for it being gone, if I had a breakpoint set just before calling updateAllItemsView, in the debugger, I could see the list array. After going through the method, it was gone, but that might be due to the program having the EXC_BAD_ACCESS?

    Thanks for all the help guys!

    Dan
     

Share This Page