2
\$\begingroup\$

I've implemented a table view controller for the purposes of subclassing. The table view controller implements a pull-to-refresh which will automatically reload the table view's data. I am looking for general feedback on my class.

DXYBaseTableViewControllerHD.h

#import "DXYBaseViewControllerHD.h"
// Displays a table view for its UI.
//
// Unlike `UITableViewController`, this controller's `view` is not the table
// view itself. This means it is possible to insert content above the table
// view.
@interface DXYBaseTableViewControllerHD : DXYBaseViewControllerHD <UITableViewDataSource, UITableViewDelegate>
// A table view controller managing `tableView` and set up as a child of the
// receiver.
@property (nonatomic, strong, readonly) UITableViewController *childTableViewController;
// The table view controlled by the receiver.
@property (nonatomic, strong, readonly) UITableView *tableView;
- (instancetype)initWithRefreshControl:(BOOL)refreshControl automaticallyLoadMore:(BOOL)loadMore;
//refreshing or loading
@property (nonatomic, assign, readonly, getter=isRefreshing) BOOL refreshing;
//trigger refresh programmatically
- (void)triggerRefresh;
//called by refreshControl or triggered programmatically
//must be overrided by subclass
- (void)doRefresh;
//called when finish refreshing
- (void)endRefreshing;
//called when scroll to bottom
//must be overrided by subclass
//when overriding scrollViewDidScroll: must call [super scroll...] first
- (void)loadMore;
//called when finish loading
- (void)endLoadingMore;
//flag when there is no more data to load
@property (nonatomic, assign, getter=isNoMoreData) BOOL noMoreData;
@end

DXYBaseTableViewControllerHD.m

#import "DXYBaseTableViewControllerHD.h"
@interface DXYBaseTableViewControllerHD ()
@property (nonatomic, assign) BOOL hasRefreshControl;
@property (nonatomic, assign) BOOL needAutoLoadMore;
@property (nonatomic, assign, getter=isLoading) BOOL loading;
@end
@implementation DXYBaseTableViewControllerHD
#pragma mark Properties
- (UITableView *)tableView {
 return self.childTableViewController.tableView;
}
#pragma mark Lifecycle
- (instancetype)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil {
 self = [super init];
 if (self == nil) return nil;
 _childTableViewController = [[UITableViewController alloc] initWithStyle:UITableViewStylePlain];
 [self addChildViewController:_childTableViewController];
 [_childTableViewController didMoveToParentViewController:self];
 return self;
}
- (instancetype)initWithRefreshControl:(BOOL)refreshControl automaticallyLoadMore:(BOOL)loadMore {
 self = [self initWithNibName:nil bundle:nil];
 if (self == nil) return nil;
 self.hasRefreshControl = refreshControl;
 self.needAutoLoadMore = loadMore;
 if (refreshControl) {
 UIRefreshControl *control = [[UIRefreshControl alloc] init];
 [control addTarget:self action:@selector(doRefresh) forControlEvents:UIControlEventValueChanged];
 self.childTableViewController.refreshControl = control;
 }
 if (loadMore) {
 self.noMoreData = NO;
 self.loading = NO;
 }
 return self;
}
- (void)viewDidLoad {
 [super viewDidLoad];
 self.tableView.frame = self.view.bounds;
 self.tableView.delegate = self;
 self.tableView.dataSource = self;
 self.tableView.backgroundColor = [UIColor DXYMainBackgroundColor];
 [self.view insertSubview:self.tableView atIndex:0];
 if (self.hasRefreshControl) {
 [self triggerRefresh];
 }
}
#pragma mark UITableViewDataSource
- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section {
 NSAssert(NO, @"This method must be overridden by subclasses");
 return 0;
}
- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
 NSAssert(NO, @"This method must be overridden by subclasses");
 return nil;
}
#pragma mark - refresh
- (BOOL)isRefreshing {
 return self.childTableViewController.refreshControl.isRefreshing || self.isLoading;
}
- (void)triggerRefresh {
 [self.childTableViewController.refreshControl beginRefreshing];
 [self doRefresh];
}
- (void)doRefresh {
 NSAssert(NO, @"This method must be overridden by subclasses");
}
- (void)endRefreshing {
 [self.childTableViewController.refreshControl endRefreshing];
}
- (void)loadMore {
 NSAssert(NO, @"This method must be overridden by subclasses");
}
- (void)endLoadingMore {
 self.loading = NO;
}
#pragma mark - scroll view delegate
- (void)scrollViewDidScroll:(UIScrollView *)scrollView {
 CGFloat offsetY = scrollView.contentOffset.y;
 CGFloat contentHeight = scrollView.contentSize.height;
 CGFloat frameHeight = scrollView.frame.size.height;
 if (self.needAutoLoadMore &&
 !self.isNoMoreData &&
 !self.isLoading &&
 (roundf(contentHeight-offsetY-frameHeight) < 44.0)) {
 self.loading = YES;
 [self loadMore];
 } 
}
@end
nhgrif
25.4k3 gold badges64 silver badges129 bronze badges
asked Apr 7, 2015 at 11:16
\$\endgroup\$
1
  • 3
    \$\begingroup\$ Can you add some description of what you class does? Are the comments actually in your source code, or just added for explanation here? \$\endgroup\$ Commented Apr 7, 2015 at 11:44

1 Answer 1

3
\$\begingroup\$

First and foremost, I have to point you to NS_REQUIRES_SUPER. This macro should be used with your methods which actually implement some logic. The ones, which if overridden, should be sure to call super, such as viewDidLoad.

Beyond that, I'm not entirely sure I'm satisfied with the design of this class. It doesn't really quite fit the design patterns we're used to seeing in iOS development. You're leaving four NSAssert() calls in production code. NSAssert is fine when testing to make sure your code is working correctly, but probably shouldn't be left in production code (leave them for unit tests?).

The NSAssert() calls feel kind of necessary though, because for our default implementation, what should we load into the table? Right? Realistically, what we need isn't a UIViewController subclass, but instead, a UIView subclass (much like UITableView is a UIView subclass). And in place of our NSAssert() calls, our subclass should have a delegate, and it should be calling required methods on our delegate in these spots.

answered Apr 8, 2015 at 0:39
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the marco it's new to me. About assert it's actually an way to make sure the specific method has been overridden by all subclasses at compile time. Instead of generating a warning as the marco do this way directly crash the app. And you're right some kind of indicator of loading process is missing \$\endgroup\$ Commented Apr 8, 2015 at 1:58
  • \$\begingroup\$ I understand what the assert is, but it's not really appropriate to leave it in production code in my opinion. Plus, subclasses won't get a warning or error at build time (only run time). If you implemented this as a view with a delegate (as UITableView is), your delegate would get a build/write time warning for not implementing it (Swift won't even compile it) and you still get the run-time exception, and you're not leaving an assert in production code. \$\endgroup\$ Commented Apr 8, 2015 at 2:15
  • \$\begingroup\$ My bad it should be "crash at run time" I mean then you'll find out during development. Whether leave assert in production code is indeed a controversial topic. Right now I just use it this way and add #ifdef DEBUG later \$\endgroup\$ Commented Apr 8, 2015 at 3:46

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.