B-Sides / ELCImagePickerController

A clone of the UIImagePickerController using the Assets Library Framework allowing for multiple asset selection
http://www.icodeblog.com
1.65k stars 469 forks source link

ELCAssetCell minor typo fix. #11

Closed adithexplorer closed 13 years ago

adithexplorer commented 13 years ago

Hi

THere is a minor typo in ELCAssetCell.m I found. When I fixed this it really sped up scrolling performance of my 400+ pics library.

Attaching code. Attaching Code

Thanks

adithexplorer commented 13 years ago

Cant Work out how this code attached system works (first timer!!!) But anyway the issue is in ELCAssetsCell.m -> Line number 17

Instead of: if(self = [super initWithStyle:UITableViewStylePlain reuseIdentifier:_identifier]) { ....

It should be if(self == [super initWithStyle:UITableViewStylePlain reuseIdentifier:_identifier]) {...

THere was a '==' missing which was hampering performance as cells were not being reused

cschep commented 13 years ago

Hello adityamatharu,

That is actually a really common idiom in objective-c. At first it looks very weird, but that's actually a compound statement.

So if the original code looks like this:

if(self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:_identifier]) {
    self.rowAssets = _assets;
}

It's assigning self to a new object, then checking that self is not nil. It's easier to see if we break that into two lines:

self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:_identifier];
if(self) {
    self.rowAssets = _assets;
}

So, it's not actually a typo. That being said, I'm definitely interested in why that sped performance up for you.

Please do a pull of that latest code (we've recently pushed some changes) and try that. If performance is still a problem, please feel free to open a new issue.

Thanks!

adithexplorer commented 13 years ago

Hi chris

Thanks for the reply.

I actually meant the == sign. I know I'm inexperienced here but I thought to compare something you needed to have a == sign rather than just a = sign. That's what I did and that improved the performance. So I'm not sure if the compound statement still works with a single = sign rather than double.

But yeah I'll checkout the new code. One thing is for certain I was getting critical crash and slowdowns Due to running out of memory when I was using only single = sign

But it's really great you replied back :)

Thanks Adi

Sent from my iPhone

On 13 Oct 2011, at 19:09, Chris Schepmanreply@reply.github.com wrote:

Hello adityamatharu,

That is actually a really common idiom in objective-c. At first it looks very weird, but that's actually a compound statement.

So if the original code looks like this:

if(self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:_identifier]) { self.rowAssets = _assets; }

It's assigning self to a new object, then checking that self in not nil. It's easier to see if we break that into two lines:

self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:_identifier] if(self) { self.rowAssets = _assets; }

So, it's not actually a typo. That being said, I'm definitely interested in why that sped performance up for you.

Please do a pull of that latest code, we've pushed some changes, and try that. If performance is still a problem, please feel free to open a new issue.

Thanks!

Reply to this email directly or view it on GitHub: https://github.com/elc/ELCImagePickerController/issues/11#issuecomment-2397981

ArjunNair commented 13 years ago

Hi Aditya,

I checked the original version of the code and could find no leaks in Instruments. At least none showed up in my testing of albums with ~400 photos. It was pretty quick on ipod 4G but ipod 2G lagged quite a bit when trying to load so many photos. But that's to be expected.

Just to clarify, the if statement simply checks the validity of the expression. if (true) (i.e not nil in this case) it does something. In the case of self = [super initWithStyle:UITableViewCellStyleDefault reuseIdentifier:_identifier] within the if statement, the self will be assigned first and if it's nil (false) it won't execute the if code block but if it's not nil (true) the code block is executed normally. As you can see, it's similar to what would happen if you first assigned to self in a statement and then used the value of self in the if expression as a condition (see Chris's second example code in his reply).

HTH.