CODEWITHSUNDEEP
iphoneobjective-c
AsTheWormTurns
AsTheWormTurns

Reputation: 1316

Reducing many similar methods to one parametrized method

I'm studying Objective-C.

A collegue left my company and I "inherited" her code. In a project, in a UIViewController class I found this code:

-(IBAction)goToSect1:(id)sender{
    sect1=[[Sect1ViewController alloc]initWithNibName:@"SECT-1" bundle:nil];
    [self presentModalViewController:sect1View animated:YES];
}

-(IBAction)goToSect2:(id)sender{
    sect2=[[Sect2ViewController alloc]initWithNibName:@"SECT-2" bundle:nil];
    [self presentModalViewController:sect2View animated:YES];
}

-(IBAction)goToSect3:(id)sender{
    sect3=[[Sect3ViewController alloc]initWithNibName:@"SECT-3" bundle:nil];
    [self presentModalViewController:sect3View animated:YES];
}

-(IBAction)goToSect4:(id)sender{
    sect4=[[Sect4ViewController alloc]initWithNibName:@"SECT-4" bundle:nil];
    [self presentModalViewController:sect4View animated:YES];
}

-(IBAction)goToSect5:(id)sender{
    sect5=[[Sect5ViewController alloc]initWithNibName:@"SECT-5" bundle:nil];
    [self presentModalViewController:sect5View animated:YES];
}

-(IBAction)goToSect6:(id)sender{
    sect6=[[Sect6ViewController alloc]initWithNibName:@"SECT-6" bundle:nil];
    [self presentModalViewController:sect6View animated:YES];
}

I consider this a little crazy, since we have six methods doing fundamentally the same thing. Is there a way to have only one parametrized method? How? Should I consider using the sender tag and a switch inside the body of my new method?

Thanks, any help is appreciated.

Upvotes: 1

Views: 75

Answers (5)

Vijay-Apple-Dev.blogspot.com
Vijay-Apple-Dev.blogspot.com

Reputation: 25692

-(IBAction)goToThisViewControllerByUsingThisSection:(id)sender{


        // Here I assume you are using this function with UIButton or other controls.

        // If so, then you have to assign a tag for each section button or do as needed by your app.

        // If you call this method in didselectrowatindexpath in delegate of UITableView then simply do use:


        //  indexPath.section instead of sender.tag

    NSString *sectionWithViewController=[NSString stringWithFormat:@"Sect%iViewController",sender.tag];

    Class classNam=NSClassFromString(sectionWithViewController);

    UIViewController *unKnownViewController=[[classNam alloc]initWithNibName:[NSString stringWithFormat:@"SECT-%i",sender.tag] bundle:nil];

    [self presentModalViewController:unKnownViewController animated:YES];



}

Upvotes: 2

Paul.s
Paul.s

Reputation: 38728

There is not much you can do as mentioned by others.

Some suggestions:

If you use appropriate naming or change your init this line can be from 

[[Sect1ViewController alloc] initWithNibName:@"SECT-1" bundle:nil];

to

[[Sect1ViewController alloc] init];

Either name the xib after the controller like Sect1ViewController or Sect1View or change your init for the class

// Sect1ViewController.m
- (id)init
{
    self = [super initWithNibName:@"SECT-1" bundle:nil];
    if (self) {
        // ...
    }
    return self
}

- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil
{
  return [self init];
}

Generally a viewController has a better idea what xib it should use than any calling code and this keep encapsulation.

May be a bad idea but included so you can see some things you can do

This structure of the method is repeating. I am not 100% you need to hold onto the viewController you are creating as the presenting controller will take a retain on it.

With this in mind we can DRY this code up slightly like so

-(IBAction)goToSect1:(id)sender{
    [self presentClass:[Sect1ViewController class]];
}

-(IBAction)goToSect2:(id)sender{
    [self presentClass:[Sect2ViewController class]];
}

-(IBAction)goToSect3:(id)sender{
    [self presentClass:[Sect3ViewController class]];
}

-(IBAction)goToSect4:(id)sender{
    [self presentClass:[Sect4ViewController class]];
}

-(IBAction)goToSect5:(id)sender{
    [self presentClass:[Sect5ViewController class]];
}

-(IBAction)goToSect6:(id)sender{
    [self presentClass:[Sect6ViewController class]];
}

- (void)presentClass:(Class)class;
{
    UIViewController *viewController = [[class alloc] init];
    [self presentModalViewController:viewController animated:YES];
    [viewController release]; viewController = nil;
}

}

This does not scale well as for every new button you need to add a new action even though they do the same thing - yuk

Upvotes: 1

ColdLogic
ColdLogic

Reputation: 7275

I wouldn't alter it at all. Those are IBActions, meaning that something in your interface is binded to them, more than likely buttons. So you have 6 buttons on your UI, each lead to a different view controller.

With the current way you have it, you can alter the individual functionality of each button without having to worry about affecting the others. Otherwise you wild be binding all those buttons to the same method, and then inside the method have to do a check for which button it is. At best, you will get rid of 5 lines of code and introduce 6 more. So the change will actually reduce readability and increase your number of lines of code.

Upvotes: 2

Oscar Gomez
Oscar Gomez

Reputation: 18488

In my opinion you should just leave it like that. The other option would be to have a single method like this:

-(IBAction)goToSection:(id)sender{
//Check the sender and then allocate depending on sender...
}

And simply bind the same method to all the buttons...

In my opinion that would look a lot worst.

Upvotes: 1

Novarg
Novarg

Reputation: 7450

try that:

-(IBAction)goToSect:(id)sender{
if (sender /*do your check here*/)
  {
    sect=[[Sect1ViewController alloc]initWithNibName:@"SECT-1" bundle:nil];
  }else if (sender /*another check*/){
    sect=[[Sect2ViewController alloc]initWithNibName:@"SECT-2" bundle:nil];
  } //other checks
  [self presentModalViewController:sect1View animated:YES];
}

you will need all these if-checks because there are different ViewControllers. If they are pretty much the same I'd just leave 1 and pass/change some parameters.

hope it helps

Upvotes: 0

Related Questions