2
\$\begingroup\$

Had my first go at implementing infinite scrolling for UITableView's in my iOS app last night, and I noticed I ended up copying/pasting quite a bit of boilerplate from one VC to the next (if I wanted infinite scroll on it).

By the way, I'm fairly new to iOS, so this question is probably more broad than just specifically keeping infinite scroll config DRY...

I have properties like these:

@property(nonatomic) int checkins_page;
@property(nonatomic) int checkins_total;
@property(nonatomic) int checkins_total_pages;
@property(nonatomic) BOOL checkins_load_more;
@property(nonatomic) int watchlist_page;
@property(nonatomic) int watchlist_total;
@property(nonatomic) int watchlist_total_pages;
@property(nonatomic) BOOL watchlist_load_more;

I have two UITableViews in this VC, so that's fun.

Then I have a delegate method:

-(void)scrollViewDidScroll:(UIScrollView *)scrollView
{

    if(scrollView == self.checkinsTableView && !self.checkins_load_more){
        return;
    }
    if(scrollView == self.watchlistTableView && !self.watchlist_load_more){
        return;
    }
    CGFloat height = scrollView.frame.size.height;

    CGFloat contentYoffset = scrollView.contentOffset.y;

    CGFloat distanceFromBottom = scrollView.contentSize.height - contentYoffset;

    if(distanceFromBottom < height)
    {
        if(scrollView == self.checkinsTableView){
            self.checkins_load_more = NO;
            if(self.checkins_page < self.checkins_total_pages){
                self.checkins_page++;
                [self downloadCheckins];
            }
        }
        if(scrollView == self.watchlistTableView){
            self.watchlist_load_more = NO;
            if(self.watchlist_page < self.watchlist_total_pages){
                self.watchlist_page++;
                [self downloadWatchlist];
            }
        }
    }
}

Now I'm seeing a lot of this stuff could be abstracted...somewhere...somehow, maybe into a more general purpose infinite scroll VC, but like I'm said, quite new to iOS, so I'm not sure the best way to go about reducing this boilerplate code.

Any advice/pointers would be appreciated on how to clean it up slightly?

\$\endgroup\$
2
\$\begingroup\$

I do not see a huge problem with the structure of this code. It is not a huge number of lines repeated, and the contents of the if structures are obviously different as far as which variables they act upon. That being said, I do believe that it can be made more clear and concise by using objects to contain some of the data.

Without seeing the whole context of the code, it looks to me like self.checkinsTableView and self.watchlistTableView are basically the same. If their contents can be converted into a class (possibly a subclass of the Apple class, possibly not, sometimes this can be tricky) then you would be able to more easily break things down into smaller methods.

You would make the following things properties of the class:

@property(nonatomic) int page;
@property(nonatomic) int total;
@property(nonatomic) int total_pages;
@property(nonatomic) BOOL load_more;

Then you would change the if statements to look like this:

if(distanceFromBottom < height)
{
    if(scrollView == self.checkinsTableView){
        [self processThing:checkinsClass]
    }
    if(scrollView == self.watchlistTableView){
        [self processThing:watchListclass];
    }
}

-(void) processThing:(YourClass *)object {
    object.load_more = NO;
    if(class.page < object.total_pages){
        object.page++;
        [self downloadList:object];
    }
}

This class could possibly be integrated into the self.watchListTableView class itself to shorten the code even further. I haven't subclassed TableView (I'm assuming that's what this is based on the name) so I don't know if it is easy/possible or not.


It might make sense to move these checks into another method for simplicity, but it is hard to say based on the context:

if(scrollView == self.checkinsTableView && !self.checkins_load_more){
    return;
}
if(scrollView == self.watchlistTableView && !self.watchlist_load_more){
    return;
}

I tried to write an example method for this, but I think it would only work easily if you subclassed the TableView, and even if you did you would have to pass in the scrollView also to perform the check, so the way that you have it might be better.


Finally I would like to talk about a couple formatting issues. Variables in Objective-C should be camelCase, so watchlist_load_more should be watchlistLoadMore.

Also I recommend more white space in a few areas. @property(nonatomic) BOOL watchlist_load_more; would look better with a space after property. Your if statements are a bit crowded. if(scrollView == self.checkinsTableView){ would read easier as if (scrollView == self.checkinsTableView) {.

Finally, I would choose one format for {} brackets and stick with it. I believe that it is fine to have the bracket either on the same line, or on the next line, but you alternate between these two options in the code. I would recommend that you pick one and be consistent with it. These are minor nitpicks, but I think they would help readability.

\$\endgroup\$
2
\$\begingroup\$

Currently you have this code because both of your table views have the same delegate, which I presume is your view controller.

This doesn't have to be the case, and in fact it's good practice to separate them in non-trivial code as it makes testing a lot easier. Given the structure of your method with its sets of checks for the identity of the tableView object, with identical logic in each case, this is a perfect candidate for extracting this logic (and the four properties you showed duplicated) into a separate delegate class. Then you can assign a separate instance of your delegate to each of your two table views, eliminating all of your if (scrollView == someInstance) checks. The method called inside the last if blocks (downloadCheckins or downloadWatchlist can simply be a property of your delegate class.

I'm not sure if you are setting your delegate in code or in a Storyboard. If it's the latter case, you will probably have to move this initialisation into code, such as in your viewDidLoad method.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.