5
\$\begingroup\$

In one of my app, i m using custom navigation bar view to handle all Navigation Bar Properties.

Is it good practice to use custom navigation in given below way? Any Optimizations need to be done. It would be helpful to me, if you have any valuable suggestions.

Source Code ---->

.h

#import <UIKit/UIKit.h>

@protocol VCNavigationBarViewDelegate <NSObject>
@optional

- (void)menuButtonPressed;
- (void)backButtonPressed;
- (void)cancelButtonPressed;
- (void)filterButonApplyPressed;
- (void)NotificationButtonPressed;
@end

typedef enum {
  NavigationStyleLeftMenu = 0,
  NavigationStyleBackButton,
  NavigationStyleCancelButton,
  NavigationStyleSearch,
  NavigationStyleFilter,
  NavigationStyleEmpty,
  NavigationStyleNotification
} NavigationStyle;

@interface VCNavigationBarView : UIView

 @property (weak, nonatomic) IBOutlet UIImageView *imgLogoConnect;
 @property (weak, nonatomic) IBOutlet UIButton *btnMenu;
 @property (weak, nonatomic) IBOutlet UIButton *btnBack;
 @property (weak, nonatomic) IBOutlet UIButton *btnCancel;
 @property (weak, nonatomic) IBOutlet UIButton *btnFilterApply;
 @property (weak, nonatomic) IBOutlet UIButton *btnFilterCancel;
 @property (weak, nonatomic) IBOutlet UILabel  *lblHeaderTitle;
 @property (weak, nonatomic) IBOutlet UIButton *btnNotification;
 @property (weak, nonatomic) id <VCNavigationBarViewDelegate> navigationDelegate;

- (IBAction)btnBackAction:(id)sender;
- (IBAction)btnMenuAction:(id)sender;
- (IBAction)btnCancelction:(id)sender;
- (IBAction)btnFilterApplyAction:(id)sender;
- (IBAction)btnNotificationAction:(id)sender;

- (void)configureWithStyle:(NavigationStyle)navigationStyle;

@end

.m

#import "VCNavigationBarView.h"
@implementation VCNavigationBarView

- (void)configureWithStyle:(NavigationStyle)navigationStyle
 {
   self.btnFilterApply.hidden = YES;
   self.btnFilterCancel.hidden = YES;
   self.btnBack.hidden = YES;
   self.btnCancel.hidden = YES;
   self.btnMenu.hidden = YES;
   self.lblHeaderTitle.hidden = YES;
   self.btnNotification.hidden = YES;
   self.imgLogoConnect.hidden = YES;

switch(navigationStyle)
{
    case NavigationStyleLeftMenu:
    {
        self.btnMenu.hidden = NO;
        self.btnNotification.hidden = NO;
        self.imgLogoConnect.hidden = NO;
    }
        break;

    case NavigationStyleBackButton:
    {
        self.lblHeaderTitle.hidden = NO;
        self.btnBack.hidden = NO;
    }
        break;

    case NavigationStyleCancelButton:
    {
        self.lblHeaderTitle.hidden = NO;
        self.btnCancel.hidden = NO;
    }
        break;

    case NavigationStyleSearch:
    {
        self.btnCancel.hidden = NO;
    }
        break;

    case NavigationStyleFilter:
    {
        self.lblHeaderTitle.hidden = NO;
        self.btnFilterApply.hidden = NO;
        self.btnFilterCancel.hidden = NO;
        self.backgroundColor = [UIColor colorWithRed:76.0/255 green:76.0/255 blue:76.0/255 alpha:1.0];
    }
        break;

    case NavigationStyleEmpty:
    {
        self.lblHeaderTitle.hidden = NO;
    }
        break;

    case NavigationStyleNotification:
    {
        self.lblHeaderTitle.hidden = NO;
        self.btnBack.hidden = NO;
    }
        break;
   }
}

- (IBAction)btnBackAction:(id)sender {
if ([self.navigationDelegate respondsToSelector:@selector(backButtonPressed)]) {
    [self.navigationDelegate backButtonPressed];
    }
}

- (IBAction)btnMenuAction:(id)sender{
if([self.navigationDelegate respondsToSelector:@selector(menuButtonPressed)])
    [self.navigationDelegate menuButtonPressed];
}

- (IBAction)btnCancelction:(id)sender{
if([self.navigationDelegate respondsToSelector:@selector(cancelButtonPressed)])
    [self.navigationDelegate cancelButtonPressed];
}

- (IBAction)btnFilterApplyAction:(id)sender{
if([self.navigationDelegate respondsToSelector:@selector(filterButonApplyPressed)])
    [self.navigationDelegate filterButonApplyPressed];
}

- (IBAction)btnNotificationAction:(id)sender{
if([self.navigationDelegate respondsToSelector:@selector(NotificationButtonPressed)])
    [self.navigationDelegate NotificationButtonPressed];
}
@end

In BaseViewController, All Other View Controller inherited from BaseViewController

BaseVC.h

@interface VCBaseViewController : UIViewController<VCNavigationBarViewDelegate>

@property (strong, nonatomic) IBOutlet UIView *navigationBarView;

- (void)configureNavigationBarWithStyle:(NavigationStyle)style;
- (void)setViewControllerTitle:(NSString *) title;

BaseVC.m

navigationBar Configuraion

 - (void)configureNavigationBarWithStyle:(NavigationStyle)style
{
   CGRect frame;
   NSArray *nibItems = [[NSBundle mainBundle] loadNibNamed:@"VCNavigationBarView" owner:nil options:nil];
   self.menuNavigationBarView = [nibItems firstObject];
   self.menuNavigationBarView.navigationDelegate = self;
   [self.menuNavigationBarView configureWithStyle:style];
   frame = self.navigationBarView.frame;
   frame.origin.y = 20.0;
   self.navigationBarView.frame = frame;

   [self.navigationBarView addSubview:self.menuNavigationBarView];
   NSArray *subConstraints;
   NSDictionary *viewsDictionary = @{@"menuNavigationBarView" : self.menuNavigationBarView };
   self.menuNavigationBarView.translatesAutoresizingMaskIntoConstraints = NO;

// Set left & right constraints to the container view
 subConstraints = [NSLayoutConstraint constraintsWithVisualFormat:@"H:|[menuNavigationBarView]|"
                                                         options:0
                                                         metrics:nil
                                                           views:viewsDictionary];
[self.navigationBarView addConstraints:subConstraints];

 // Set left & right constraints to the container view
 subConstraints = [NSLayoutConstraint constraintsWithVisualFormat:@"V:|[menuNavigationBarView]|"
                                                         options:0
                                                         metrics:nil
                                                           views:viewsDictionary];
[self.navigationBarView addConstraints:subConstraints];
}

- (void) setViewControllerTitle:(NSString *) title
{
  [self.menuNavigationBarView.lblHeaderTitle setText:title];
}

In Child/Inherited View Controller from BaseViewController

.m

- (void)viewDidLoad
{
 [super viewDidLoad];
 [self configureNavigationBarWithStyle:NavigationStyleBackButton];
 [self setViewControllerTitle:@"My Orders"];
}
\$\endgroup\$

1 Answer 1

7
\$\begingroup\$

There are piles of formatting problems in this code which should be the first thing you strive to fix.


You have inconsistent brace styles.

Your IBAction methods have same-line braces. This is the preferred style of Xcode. This is how Xcode would have generated these methods automatically when you hooked them up.

Your other methods (and other constructs which require braces) have mostly used the new-line brace style.

It doesn't matter which you choose. What matters is consistency. And when you're writing in Xcode, the same-line brace style we see in your IBAction methods will be the easiest, because that's how Xcode will generate any methods automatically, and that's how Xcode will autocomplete methods from the superclass or protocols, and that's how Xcode will autocomplete things like for loops.

And in some cases, you've completely omitted braces, which you should never do.


You have inconsistent indentation.

By my count, things are indented by 0, 1, 2, 3, and 4 spaces, depending on where you look. You should pick one and stick to it. And make sure everything is indented the appropriate amount. Xcode will even completely do this for you. Start from the very top of your code, go to the first line that should be indented, backspace to the beginning of the line (Cmd+Backspace), and then push Tab once. Xcode knows exactly how far to indent it to get it to all line up.

There is a setting in Xcode that will let you specify how many spaces per indentation level.

enter image description here

The default setting is exactly as pictured here, and therefore, most all Xcode developers will have these settings. I recommend sticking with these settings unless you have a very compelling reason to use otherwise. And then, all that's left is to stop fighting Xcode and let it indent how it wants to.


Other notes...

Your IBOutlets and IBActions have no business in the header file. They're not open for public interaction (or shouldn't be) and as such, shouldn't be exposed to the public.

Your second comment here:

// Set left & right constraints to the container view

is in accurate. You have the same comment twice. One is above setting horizontal constraints, but the second is above setting vertical constraints.

Speaking of these constraints, it is exceptionally strange to waste time setting frames manually only to follow up by setting constraints. We really shouldn't be setting any constraints manually at all these days. Stick to setting up constraints properly. (And presumably, the 20 is to account for the status bar... but there's a way to constrain to this directly rather than hardcoding its height and hoping it doesn't change in iOS10+ [I don't know the way off the top of my head, but there's a way]).

There's no reason for the last method in your protocol (or any method) to start with a capital letter. It should be lower case. I know the Code Review syntax highlighting isn't perfect, but the fact that it did weird things for just that method should be a clue here.

If there is a means to set a property on an object, there should be a way to check that property. We don't expose a method to find out what the current title is set to.

We've not exploited Objective-C nullability annotations anywhere in our code. We should definitely be doing this, not only so that our code can be used from Swift without introducing implicitly unwrapped optionals (which are terrible), but also so that even in Objective-C, our code is more self-documenting. We have told users whether return values and arguments can be nil or should not be nil.

\$\endgroup\$
0

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.