Crash Fix #24
Crash Fix #24
Conversation
| indexPaths.append(IndexPath(row: i, section: 0)) | ||
| } | ||
| //self.dataSource.newsCount() < NewsViewUX.MinNewsCellsCount crashes the app | ||
| if self.dataSource.newsCount() > NewsViewUX.MinNewsCellsCount { |
naira-cliqz
Apr 16, 2018
Contributor
I would suggest another solution. It needs only to be changed "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount() > 0 ? 0 : self.dataSource.newsCount())..<self.dataSource.newsCount() { ... "
It will solve the crash and also in the case if newsCount is less than minNewsCount, but > 0, it will show that news...
I would suggest another solution. It needs only to be changed "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount() > 0 ? 0 : self.dataSource.newsCount())..<self.dataSource.newsCount() { ... "
It will solve the crash and also in the case if newsCount is less than minNewsCount, but > 0, it will show that news...
timoteipalade
Apr 17, 2018
•
Author
Contributor
@naira-cliqz my understanding is that getExtraNewsIndexPaths() only adds to the cells that are already visible. So in the case where newsCount is less than minNewsCount, there is nothing to add, but news will still be visible.
@naira-cliqz my understanding is that getExtraNewsIndexPaths() only adds to the cells that are already visible. So in the case where newsCount is less than minNewsCount, there is nothing to add, but news will still be visible.
naira-cliqz
Apr 17, 2018
Contributor
That's true, I thought it's a general method to get all rows. But then it can be "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount()) ..< self.dataSource.newsCount() ..." without if statement...
That's true, I thought it's a general method to get all rows. But then it can be "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount()) ..< self.dataSource.newsCount() ..." without if statement...
timoteipalade
Apr 17, 2018
Author
Contributor
You are embedding the if statement in the for statement. It is cleaner if we keep them separated. They are easier to read.
You are embedding the if statement in the for statement. It is cleaner if we keep them separated. They are easier to read.
timoteipalade
Apr 17, 2018
•
Author
Contributor
My bad. I thought it was the same code as above. Technically, min is an if too :).
My bad. I thought it was the same code as above. Technically, min is an if too :).
timoteipalade
Apr 17, 2018
•
Author
Contributor
I want to leave the code like this. It is easier to reason about. I can easily see what is guaranteed inside the if. With min you have to think...oh hey if the min of the 2 is the newsCount, then the for will not be executed. So you have to think of a special case of the min to understand what is happening. With an if, that is not necessary. I think code is elegant when it is short and easy to understand.
I want to leave the code like this. It is easier to reason about. I can easily see what is guaranteed inside the if. With min you have to think...oh hey if the min of the 2 is the newsCount, then the for will not be executed. So you have to think of a special case of the min to understand what is happening. With an if, that is not necessary. I think code is elegant when it is short and easy to understand.
mahmoud-adam85
Apr 18, 2018
Contributor
@naira-cliqz I think using if before the for loop is more readable than embed min in the for loop
@naira-cliqz I think using if before the for loop is more readable than embed min in the for loop
|
That's true, I thought it's a general method to get all rows. But then it can be "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount()) ..< self.dataSource.newsCount() ..." without if statement... |
| indexPaths.append(IndexPath(row: i, section: 0)) | ||
| } | ||
| //self.dataSource.newsCount() < NewsViewUX.MinNewsCellsCount crashes the app | ||
| if self.dataSource.newsCount() > NewsViewUX.MinNewsCellsCount { |
naira-cliqz
Apr 17, 2018
Contributor
That's true, I thought it's a general method to get all rows. But then it can be "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount()) ..< self.dataSource.newsCount() ..." without if statement...
That's true, I thought it's a general method to get all rows. But then it can be "for i in min(NewsViewUX.MinNewsCellsCount, self.dataSource.newsCount()) ..< self.dataSource.newsCount() ..." without if statement...
|
min is not a complicated and the code is shorter and more elegant. |
No description provided.