display image from xml

[sangau001]

HI,
I am parsing an xml and getting url of image from that xml.
Now I want to display that image from url.
This is my code

- (void)setHomeHome *)newHome
{
[newHome retain];
[_home release];
_home = newHome;

self.bedsLabel.text = newHome.details ;
self.cszLabel.text = newHome.csz;
self.addressLabel.text = newHome.address;
self.price.text = newHome.price;
self.propImageView.image = [self imageFromURL:newHome.picture];
[self setNeedsDisplay];
}

- (UIImage *)imageFromURLNSString *)url {
//NSString *temp = @"http://imgprd.nrtwebservices.com/California/properties/JPG_Main_White/222/7321C37C-80B4-43FD-B4E3-A01AF52B7222_9.jpg";
//The above code which is commented works fine for me when I give direct Image URL
NSString *temp = url;//But when I try this this doesnt work.

NSURL *url1 = [NSURL URLWithString:temp];
NSData *data = [NSData dataWithContentsOfURL:url1];
UIImage *propImage = [[UIImage alloc] initWithData:data cache:NO];
return propImage;
}


Any Idea what is going on?
If anybody can help me plz.
Thanks.

[bonehead]

What is the error you're getting?

Have you checked to see whether newHome.picture is a complete URL?

[jeff_lamarche]

Did you try using NSlog to print out the value of the url, or did you look at its value in the debugger? Maybe it got messed up in the parsing, maybe there's a special character that needs to be url-escaped, etc. It could be any number of things - you'll need to provide more info before anybody can help you here, I'm afraid. Also, are you checking url1 to make sure that it's creating a valid URL?

And a few general comments about your code:

Why are you assigning url to a stack local variable? I don't see any reason for doing that. It makes your code harder to read and uses unnecessary space on the stack. Also, you're leaking here, you allocated memory for propsImage, but you never release it. You are also using convenience methods, which is generally a no-no on the iPhone because they require using the autorelease pool unnecessarily.

Lastly, although this is picayune, I would use more descriptive variable names. Don't call an NSString variable "url", that's potentially confusing a few months from now when you're debugging this code and haven't looked at it in months.

Here's a stab at a quick rewrite of imageFromURL:, although, I'm actually not convinced that this should be in a method at all. By putting this functionality in a method rather than doing it inline, you're forced to put at least one thing in the autorelease pool, which is something you want to avoid on the iPhone because it will not use the swap file for volatile memory, meaning you are very much memory constrained and it doesn't take a whole lot of leaked image files to use up the available memory - on current generations of the iPhone, your application can expect best case scenario of 60-80 megs of RAM for its use. Call your original method a few dozen times and you're in a low-memory situations and your app will crash. (Remember, Instruments is your friend - it has a template to find leaks like this).

- (UIImage *)imageFromURL:(NSString *)urlString {
	NSURL *url = [[NSURL alloc] initWithString:urlString];
	NSData *data = [[NSData  alloc] initWithContentsOfURL:url];
	[url release];
	UIImage *propImage = [[UIImage alloc] initWithData:data];
	[data release];
	return [propImage autorelease];
}

So, that's better then what you have, but I think you're even better off doing it like this inline in your setHome method like this (remember, I can't compile this code, so there might be some minor issues with it):

- (void)setHome:(Home *)newHome
{
	[newHome retain];
	[_home release];
	_home = newHome;
	
	self.bedsLabel.text = newHome.details ;
	self.cszLabel.text = newHome.csz;
	self.addressLabel.text = newHome.address;
	self.price.text = newHome.price;
	NSData *data = [[NSData  alloc] initWithContentsOfURL:self.picture];
	[url release];
	UIImage *propImage = [[UIImage alloc] initWithData:data];
	[data release];
	self.propImageView.image = propImage;
	[propImage release];
	[self setNeedsDisplay];
}

One last thing - I have two comments about this line of code:

 [self setNeedsDisplay]

First, setNeedsDisplay is VERY costly on the iPhone. Unless this change affects the vast majority of the view (and it might), you should use setNeedsDisplayInRect: instead of just setNeedsDisplay. The display algorithms on the iPhone are optimized for partial redraws.

Secondly, this looks like classic model or controller code (it's a grey area, I could see arguments for both) - but it looks like you're doing it inside the mutator of a UIView subclass, meaning you're doing it in the one place you probably shouldn't be according to the tenants of MVC - in the View component, or else you don't have a clear distinction between model and view components which will make it harder to maintain your code down the line.

Now I could be missing something here, since since I'm working from two methods completely out of context), but your code seems to violate MVC, so you might want to take a step back and just make sure you're happy with the overall design of your code.

I hope this helps. I know it's not specifically the problem you're looking to get fixed, but some of these things - especially the memory leaks - will bite you down the line. Let us know what you find in the debugger, and we can maybe get your original problem resolved too.