1

I'm writing an app for iOS and i have to read an external xml file. My program logic is working very well. I checked that several times with NSLog messages. My problem is, when I'm adding an new object to my NSMutuableArray masterNewsList then is after the insertion every object overwritten by the last inserted object. What's my mistake here? I can't find it.

//  NewsData.h

#import <Foundation/Foundation.h>

@interface NewsData : NSObject

@property (nonatomic,copy) NSString *title;
@property (nonatomic,copy) NSString *date;
@property (nonatomic,copy) NSString *detail;
@property (nonatomic,copy) NSString *content;

-(id) initWithTitle:(NSString *)title  date:(NSString *)date detail:(NSString *)detail content:(NSString *)content;

@end


//NewsData.m

#import "NewsData.h"

@implementation NewsData

-(id)initWithTitle:(NSString *)title date:(NSString *)date detail:(NSString *)detail content:(NSString *)content{
 self = [super init];
 if (self) {
    _title = title;
    _date = date;
    _detail = detail;
    _content = content;
    return self;
}
return nil;
}

@end



//  NewsDataController.h


#import <Foundation/Foundation.h>

@class NewsData;

@interface NewsDataController : NSObject <NSXMLParserDelegate>

@property (nonatomic, copy) NSMutableArray *masterNewsList;

- (NSUInteger)countOfList;
- (NewsData *)objectInListAtIndex:(NSUInteger)theIndex;

@end



//  NewsDataController.m


#import "NewsDataController.h"
#import "NewsData.h"

@interface NewsDataController()

@property NSMutableString *title;
@property NSMutableString *description;
@property NSMutableString *content;
@property NSMutableString *date;

@property BOOL itemValue;
@property BOOL titleValue;
@property BOOL descriptionValue;
@property BOOL contentValue;
@property BOOL dateValue;

-(void) initializeDataList;
- (void)addNewsData:(NewsData *)newsData;

@end

@implementation NewsDataController

- (void) initializeDataList {
    NSMutableArray *newsList = [NSMutableArray array];
    self.masterNewsList = newsList;

    self.title = [NSMutableString stringWithString:@""];
    self.description = [NSMutableString stringWithString:@""];
    self.content = [NSMutableString stringWithString:@""];
    self.date = [NSMutableString stringWithString:@""];

    self.itemValue = false;
    self.contentValue = false;
    self.dateValue = false;
    self.titleValue = false;
    self.descriptionValue = false;

    NSData *xmlData = nil;
    xmlData = [[NSData alloc] initWithContentsOfURL:[NSURL URLWithString:@"http://www.somesite.de/?type=100"]];
    if (xmlData != nil) {
        NSXMLParser *theParser = [[NSXMLParser alloc] initWithData:xmlData];
        theParser.delegate = self;
        [theParser parse];
    }
     else{
        NewsData *newsData;
        newsData = [[NewsData alloc] initWithTitle:@"Es konnten keine News geladen werden" date: @"---" detail:@"Keine Verbindung zum Server" content:@"Bitte Netzwerkverbindung überprüfen!"];
        [self addNewsData:newsData];
    }
}

-(void) setMasterNewsList:(NSMutableArray *)newList{
    if (_masterNewsList != newList) {
        _masterNewsList = [newList mutableCopy];
    }
}

-(id) init{
    if (self = [super init]) {
        [self initializeDataList];
        return self;
    }
    return nil;
}

- (NSUInteger) countOfList{

    return [self.masterNewsList count];
}

- (NewsData *)objectInListAtIndex:(NSUInteger)theIndex{

    return [self.masterNewsList objectAtIndex:theIndex];
}

Method who add the object to masterNewsList

- (void) addNewsData:(NewsData *)newsData{

    [self.masterNewsList addObject:newsData];

}

-(void) parser:(NSXMLParser *)parser didStartElement:(NSString *)elementName namespaceURI:(NSString *)namespaceURI qualifiedName:(NSString *)qName attributes:(NSDictionary *)attributeDict{

    if ([elementName isEqualToString:@"item"]) {
        self.itemValue = true;
    }
    if ([elementName isEqualToString:@"title"]) {
        [self.title deleteCharactersInRange:NSMakeRange(0, self.title.length)];
        self.titleValue = true;
    }
    if ([elementName isEqualToString:@"description"]) {
        [self.description deleteCharactersInRange:NSMakeRange(0, self.description.length)];
        self.descriptionValue = true;
    }
    if ([elementName isEqualToString:@"content:encoded"]) {
        [self.content deleteCharactersInRange:NSMakeRange(0, self.content.length)];
        self.contentValue = true;
    }
    if ([elementName isEqualToString:@"pubDate"]) {
        [self.date deleteCharactersInRange:NSMakeRange(0, self.date.length)];
        self.dateValue = true;
    }
 }

Here I'm creating the new object of kind NewsData and calling then the addNewsData Method

-(void) parser:(NSXMLParser *)parser didEndElement:(NSString *)elementName namespaceURI:(NSString *)namespaceURI qualifiedName:(NSString *)qName{
    if ([elementName isEqualToString:@"item"]) {
        NewsData *newsData;
        newsData = [[NewsData alloc] initWithTitle:self.title date:self.date detail:self.description content:self.content];
        [self addNewsData:newsData];
        self.itemValue = false;
    }
    if ([elementName isEqualToString:@"title"]) {
        self.titleValue = false;
    }
    if ([elementName isEqualToString:@"description"]) {
        self.descriptionValue = false;
    }
    if ([elementName isEqualToString:@"content:encoded"]) {
        self.contentValue = false;
    }
    if ([elementName isEqualToString:@"pubDate"]) {
        self.dateValue = false;
    }
}

-(void) parser:(NSXMLParser *)parser foundCharacters:(NSString *)string{
    if (self.itemValue && self.titleValue) {
        [self.title appendString:string];
    }
    if (self.itemValue && self.descriptionValue) {
        [self.description appendString:string];
    }
    if (self.itemValue && self.contentValue) {
        [self.content appendString:string];
    }
    if (self.itemValue && self.dateValue) {
        [self.date appendString:string];
    }
}

-(void) parser:(NSXMLParser *)parser foundCDATA:(NSData *)CDATABlock{
    if (self.itemValue && self.contentValue) {
        [self.content appendString:[[NSString alloc] initWithData:CDATABlock encoding:NSUTF8StringEncoding]];
    }
}



@end

2 Answers2

1

Your mistake is here:

if (self) {
    // None of these assignments copies the incoming mutable strings.
    // When strings change later on, so do titles, details, content, and so on.
    _title = title;
    _date = date;
    _detail = detail;
    _content = content;
    return self;
}

You are using assignments to backing variables of properties marked copy. Switch to assigning to properties, and your problem will be fixed:

if (self) {
    // Since your property is correctly marked `copy` (a good idea for NSString)
    // these assignments will make copies of mutable strings,
    // preventing the unwanted modifications.
    self.title = title;
    self.date = date;
    self.detail = detail;
    self.content = content;
    return self;
}
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Thank you very much! I'm relatively new to iOS development and this was really a mistake from a noob ;) – user2025039 Jan 31 '13 at 14:26
  • Although probably fine in this case, isn't it bad practice to use getters and setters in an init method? In which case `_title = [title copy];` should work. – James P Jan 31 '13 at 14:28
  • @user2025039 You are welcome! Mistakes like that are relatively easy to spot: as soon as you see something changing behind your back, you know that you have a mutable reference that has been shared unintentionally. – Sergey Kalinichenko Jan 31 '13 at 14:28
  • @JamesP Yes, "no accessors in initializers and deallocs" is a rather popular opinion. However, I agree with [a couple of highly respected sources on Stack Overflow](http://stackoverflow.com/q/1283419/335858) that using accessors in `init` is nearly always a good idea ([`dealloc` not so much](http://stackoverflow.com/q/1283419/335858)). – Sergey Kalinichenko Jan 31 '13 at 14:37
  • I'm not sure I get their reasoning, other than the fact that you don't have to remember to retain. There points also seem very out of date (check the comments). I think it depends on your app and if you know exactly what's going on. But I think it is still safer to set the ivar directly, you never know what someone else is going to do with your code. – James P Jan 31 '13 at 14:48
0

I'm not entirely sure what could be causing this problem, but I STRONGLY suggest you look for DDXML and use that small set of classes to work with xml. They provide simple getters for nodes and childs and stuff, and it's a hell of a lot easier to work with that than parsing the XML yourself.

Cheers

Ismael
  • 3,927
  • 3
  • 15
  • 23
  • Thank you for your tip. I will keep that in mind, maybe it will be useful for a future project. Here is the file so simple, that I decided to parse it by my own. – user2025039 Jan 31 '13 at 15:33