0
\$\begingroup\$

In UITableViewCell class, I have two properties:

  1. UINavigationController

    @interface TableViewCell
    @property (nonatomic, strong) UINavigationController * targetNaviCtrl;
    
    @implementation TableViewCell
    - (UINavigationController *)targetNaviCtrl{
        if(!_targetNaviCtrl){
             UIResponder *target = self.nextResponder;
             do {
             target = target.nextResponder;
             } while (![target isKindOfClass:UINavigationController.self] && 
                   target != nil);
           _targetNaviCtrl = (UINavigationController *)target;
         }
       return _targetNaviCtrl;
    }// I use nextResponder to find its parent UINavigationController
    

    Find parent UINavigationController of the cell, and use the controller.

    While every cell is dequeued and created:

    TableViewCell *cell = [tableView dequeueReusableCellWithIdentifier: kCellIdentifier forIndexPath: indexPath];
    

    So I think the property targetNaviCtrl is being created all the time (about a screen and a half).

    I do not want to repeat calculating the same. I think the property targetNaviCtrl is static,

    How can I improve it?

  2. View of menu:

    @interface TableViewCell
    @property (nonatomic,strong) MenuView *menuView;
    

    I call up the menu view:

    @implementation TableViewCell
    - (IBAction)menuButtonClick:(UIButton *)sender {
        [_menuView mas_makeConstraints:^(MASConstraintMaker *make) {
            make.width.mas_equalTo(205);
            make.height.mas_equalTo(60);
            make.bottom.mas_equalTo(self.menuButton).offset(0);
            make.right.mas_equalTo(self.menuButton.mas_left).offset(-10);
        }];
    }
    

    I put the cell's job in a category:

    @implementation ZBMyProductsCell (Actions)
    
    - (void)editMyProduct{
    
        PutOnShelvesViewController *putOnShelvesVC = [PutOnShelvesViewController new];
        putOnShelvesVC.product = self.myProduct;
        [self.targetNaviCtrl pushViewController:putOnShelvesVC animated:YES];     
    }
    

    The menu view's implementation:

    @implementation MenuView
    {
        UIButton *_deleteButton;
        UIButton *_editButton;
        UIButton *_forwardButton;
    }
    
    static MenuView *_instance;
    + (instancetype)sharedInstance{
    
        static dispatch_once_t onceToken;
        dispatch_once(&onceToken, ^{
            _instance = [[self alloc] initWithFrame:CGRectZero];
        });
        return _instance;
    }
    

    img

    The menuView could only be held by one cell. And the menuView shall be part of the cell. It shall not be a overall singleton.

    How can I do it better?

Why do the cells need access to the navigation controller? I want to decouple the ViewController, according to WWDC Advanced User Interfaces with Collection Views and ObjC.io Lighter View Controllers.

I separated out data source, and try to do as AggregateDataSource. And I use MVCS pattern to lighter the view controller.

Here is a Store's delegate methods. I request the data, give it to the dataSource:

@implementation ProductsViewController
- (void)networkProductsStoreSuccess:(NSArray<Product *> *)products productCount:(NSUInteger) count{
        WeakSelf(weakSelf);
        self.productsDataSource = [[ProductsDataSource alloc] initWithItems: products cellIdentifier: kProductCell configureCellBlock:^(TableViewCell *cell, Product *product, BOOL isCellContained, NSIndexPath *indexPath) {
            if (isCellContained && self.tableView.editing) {
                [self.tableView selectRowAtIndexPath: indexPath animated:YES scrollPosition:(UITableViewScrollPositionNone)];
            }
        }];
        self.tableView.dataSource = self.productsCtrlDataSource;

If I assign the cell's action delegate to the ViewController.

Then the code:

 @implementation ProductsViewController
-(void)networkProductsStoreSuccess:(NSArray<Product *> *)products productCount:(NSUInteger) count{
        WeakSelf(weakSelf);
        self.productsDataSource = [[ProductsDataSource alloc] initWithItems: products cellIdentifier: kProductCell configureCellBlock:^(TableViewCell *cell, Product *product, BOOL isCellContained, NSIndexPath *indexPath) {
        cell.actionDelegate = weakSelf;// Added code.
            if (isCellContained && self.tableView.editing) {
                [self.tableView selectRowAtIndexPath: indexPath animated:YES scrollPosition:(UITableViewScrollPositionNone)];
            }
        }];
        self.tableView.dataSource = self.productsCtrlDataSource;

I seldom see code like this (assign delegate asynchronously).

If I assign the cell's action delegate to the DataSource. Is it more natural?

Turn

@interface ProductsDataSource : NSObject<UITableViewDataSource>

To

@interface ProductsDataSource : NSObject<UITableViewDataSource, CellActionDelegate>
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Why do the cells need access to the navigation controller? \$\endgroup\$
    – Martin R
    Jan 7, 2018 at 13:04
  • \$\begingroup\$ Because the menu actions of the cell, include pushViewController. And I updated the code \$\endgroup\$
    – dengApro
    Jan 7, 2018 at 14:11
  • \$\begingroup\$ @Martin R the great kind engineer \$\endgroup\$
    – dengApro
    Jan 7, 2018 at 15:02

2 Answers 2

0
\$\begingroup\$

Omg a lowly TableViewCell does not tell a NavigationController what to do!

Give your cell delegate methods like productCell:didTapEditButton: and set your VC as the cell delegate.

Now the cell is just passing along the message 'my edit button was tapped' and the VC can decide what to do about that. - ie configure and push a new VC

\$\endgroup\$
1
  • \$\begingroup\$ Typical solution \$\endgroup\$
    – dengApro
    May 29, 2018 at 1:45
1
\$\begingroup\$

You are mixing controller logic into view. Breaking single responsibility principle.

Also I think you are abusing Singleton pattern. Menu view does not have to be a singleton. Having it as a singleton will do more harm than good in your case. What is more, underscore is a prefix for ivars not static var.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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