Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have been trying to achieve the below requirements to get them working. Am I doing it the right way?

  1. We have a table with customised Cell (say: firstViewController)

  2. Customised Cell has a button, click event in this button, and carries row index value to another view controller. In this view controller I will use this row index to access the data model from first view controller.

Annotation: Each row has values associated with unique Dictionary value in Array based on row Index.

Pseudo Code: (pasted only necessary code)

FirstViewController.h

 @interface firstViewController : UIViewController <UITableViewDataSource, UITableViewDelegate>


 @property NSMutableArray *MasterRecords;   //  Each element in Array is dictionary. 
 @property (weak, nonatomic) IBOutlet UITableView *tblMasterTable;
 @end

FirstViewController.m

-(UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{

    static NSString *columnIdfer = @"items";
    StudentCell *cell = (StudentCell *)[tableView dequeueReusableCellWithIdentifier:columnIdfer];
    if ( cell == nil)
    {
        NSArray *nib = [[NSBundle mainBundle] loadNibNamed:@"StudentCell" owner:self options:nil];
        cell = [nib objectAtIndex:0];
    }


    if (indexPath.row < [self.MasterRecords count])
    {

        cell.iRecordIndex = indexPath.row;
    }


    return cell;

}

In the above code I set the iRecordIndex to IndexPath.row

StudentCell.h

@interface StudentCell : UITableViewCell
@property NSInteger iRecordIndex;
@property (weak, nonatomic) IBOutlet UIButton *btnSubmit;  // It pushes viewController.
@end

StudentCell.m

- (IBAction)btnSubmitPressed:(id)sender {

     NextViewController *sVC = [[NextViewController alloc]     initWithRecIndex:@"NextViewController" bundle:nil Index:self.iRecordIndex];

  // Pushing NextViewController to NavigationViewController.
}

NextViewController.h

@interface NextViewController : UIViewController <MKMapViewDelegate>
- (id)initWithRecIndex:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil  Index:(NSInteger )iRecIndex;
@property NSInteger iRecordIndex;
@end

NextViewController.m

 - (id)initWithRecIndex:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil  Index:(NSInteger )iRecIndex
{
      self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self) {
        // Custom initialization
        self.iRecordIndex = iRecIndex;
    }

    return self;
}

-(void) viewWillAppear:(BOOL)animated
{
    //Get FistViewController.   say  ==> fvc.

    NSMutableArray *mRec = [fvc MasterRecords];
    NSMutableDictionary *mDict = [mRec objectAtIndex:self.iRecordIndex];

    NSLog (@" value1 is %@ ", [mDict valueForKey:@"value1"]);
    NSLog (@" value2 is %@ ", [mDict valueForKey:@"value2"]);

   // Using value1, and value2 to display MKMapView.
}

Is this the right approach, or should I improve it?

share|improve this question

closed as off-topic by 200_success Jul 25 at 23:32

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions must involve real code that you own or maintain. Questions seeking an explanation of someone else's code are off-topic. Pseudocode, hypothetical code, or stub code should be replaced by a concrete example." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

2  
    
Makes sense. Surely will take care!! –  Whoami Jul 28 at 5:05
    
@Whoami The recommended course of action would be to improve your question so it can be reopened. –  nhgrif Jul 28 at 16:26

2 Answers 2

There are several problems with your code.

First, you should use more descriptive names for your instance variables/properties.

 @interface firstViewController : UIViewController <UITableViewDataSource, UITableViewDelegate>

Class names in Cocoa are conventionally named with a prefix in capitals followed by the name of the class in TitleCase.

 @property NSMutableArray *MasterRecords;   //  Each element in Array is dictionary. 

Names of instance variables and properties should be in camelCase.


    static NSString *columnIdfer = @"items";
StudentCell *cell = (StudentCell *)[tableView dequeueReusableCellWithIdentifier:columnIdfer];
if ( cell == nil)
{
    NSArray *nib = [[NSBundle mainBundle] loadNibNamed:@"StudentCell" owner:self options:nil];
    cell = [nib objectAtIndex:0];
}

You should register the nib of your cell before using your table view. This eliminates the need for instantiating a cell yourself every time it's need, and also does away with if (cell == nil), as dequeueReusableCellWithIdentifier: will always return a valid cell, provided you set the table view up properly. Proper code with these things in mind would be:

- (void)viewDidLoad
{
    [super viewDidLoad];
    . . .
    [self.tblMasterTable registerNib:[UINib nibWithNibName:@"StudentCell" bundle:nil];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    StudentCell *cell = (StudentCell *)[tableView dequeueReusableCellWithIdentifier:@"items"];
    . . .
    return cell;
}

- (IBAction)btnSubmitPressed:(id)sender {
    NextViewController *sVC = [[NextViewController alloc]     initWithRecIndex:@"NextViewController" bundle:nil Index:self.iRecordIndex];
    // Pushing NextViewController to NavigationViewController.
}

Following the MVC pattern, your view should know nothing about your controllers. You could reimplement the above using a view controller as the cell's delegate, passing a suitable identifier in a suitable method. The delegate view controller could then manage instantiating and presenting/pushing other view controllers.


- (id)initWithRecIndex:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil  Index:(NSInteger )iRecIndex;

Several problems with this. You're specifying the return type as id, but I assume it always returns a NextViewController. As suggested in the comments, current practice is to use instancetype as the return type instead of id. For more information on id vs instancetype check this answer out.

You do not attempt to make a distinction between RecIndex and Index in the method name. The argument names do not match what the method name tells us. The implementation of this method doesn't help matters.


-(void) viewWillAppear:(BOOL)animated
{
//Get FistViewController.   say  ==> fvc.

NSMutableArray *mRec = [fvc MasterRecords];
NSMutableDictionary *mDict = [mRec objectAtIndex:self.iRecordIndex];

NSLog (@" value1 is %@ ", [mDict valueForKey:@"value1"]);
NSLog (@" value2 is %@ ", [mDict valueForKey:@"value2"]);

// Using value1, and value2 to display MKMapView.
}

Inconsistent spacing. Make up your mind about how you want to space out your method implementations/declarations and use that style consistently throughout your code.

CodeReview does not appreciate pseudo-code. Please post all relevant bits of code.

Again, instance variables are badly named. Use more descriptive names.

This view controller should not know much about it's presenting view controller.

It should declare properties for the information it needs to get from its presenting view controller. The presenting view controller can then set these properties up after instantiating but before presenting this view controller.

NextViewController *vc = . . .;
vc.mRec = . . .;

[self.navigationController pushViewController:vc animated:YES];

Or in the prepareForSegue: method if using storyboards.

share|improve this answer
    
It is common practice for init methods to return id or instanceType. –  CrimsonChris Jul 25 at 18:42
    
The return type of init methods should be instancetype unless you are intentionally preventing subclassing. –  nhgrif Jul 25 at 21:29
    
@CrimsonChris I prefer instancetype over id, due to the reasons mentioned here. –  duci9y Jul 25 at 22:05
    
I use instancetype myself. I just don't agree that id should be considered "wrong" until Xcode starts defaulting to it. Also, if I'm not mistaken all init methods that return id are auto-magically upgraded to instancetype by the compiler. –  CrimsonChris Jul 26 at 0:58
    
@CrimsonChris Yes, all ids are changed to instancetypes by the compiler. I prefer instancetype to be visible in the code, so that what you see is what actually gets compiled and run. I am not saying id is wrong, just discouraging OP from using it. –  duci9y Jul 26 at 6:02

Your "next view controller" shouldn't have to know anything about the view controller presenting it ("first view controller"). You should consider just passing your view model to the "next view controller". Also, consider passing something more concrete than just an NSDictionary as your view model.

View models are used to represent the thing you are displaying in the view layer. They typically look something like this.

@interface CatViewModel : NSObject
@property (nonatomic, strong) NSString *name;
@property (nonatomic, strong) NSString *breed;
@property (nonatomic, strong) NSDate *dateOfBirth;
@property (nonatomic, strong) NSArray* treats;
@property (nonatomic) BOOL canHasCheezBurger;
@end
share|improve this answer

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