5
\$\begingroup\$

I have an iOS app that support all orientations. I sometime notices performance issues when I rotate the device, and sometimes it crashes, badly. And it's not as smooth as other apps. This is my first app, so I'm not a pro. I have quite a lot of custom UI elements so most of my views is done programatically. The issue only occurs in one of my view controllers, PageViewController.

Since I'm not 100% sure where the problem is located, I will paste quite a lot of code. Other refactoring tips would therefore also be appreciated.

How I handle rotations:

// Suports all directions.
- (BOOL)shouldAutorotateToInterfaceOrientation (UIInterfaceOrientation)interfaceOrientation {
    return YES;
}   

// Next and prev buttons is always visible in landscape mode.
// This method sets their frames/locations correctly.
// (This method should have a better name since it also handles undo/redo buttons:)
- (void)setNextAndPrevButtonsInLandscapeMode
{
    [self enableAndDisableNavigationButtons];
    [self enableAndDisableUndoAndRedoButtons];

    previousButton.frame = CGRectMake(-50, 200, 60, 60);
    nextButton.frame = CGRectMake(300, 200, 60, 60);
    undoButton.frame = CGRectMake(-50, 120, 60, 60);
    redoButton.frame = CGRectMake(300, 120, 60, 60); 
    if (editing == NO) {
        [self.view addSubview:previousButton];
        [self.view addSubview:nextButton];
        [self.view addSubview:undoButton];
        [self.view addSubview:redoButton];
    }
}

// Method for handeling orientation change.
- (void)handleOrientation:(UIInterfaceOrientation)orientation
{
    if (orientation == UIInterfaceOrientationPortrait || orientation ==   UIInterfaceOrientationPortraitUpsideDown) {
        toolImageView.frame = CGRectMake(self.view.frame.size.width - 70, 12,   toolImageView.frame.size.width, toolImageView.frame.size.height);

        if (editing) {
            pageView.bounds = editingPortraitPaperRect;; 
            bottomMenu.frame = editingPortraitMenuRect;
        } else {
            bottomMenu.frame = defaultPortraitMenuRect;
        } 

        self.pageView.transform = CGAffineTransformIdentity;
        self.pageView.transform = CGAffineTransformMakeRotation(M_PI * (0) / 180.0);
        self.pageView.transform = CGAffineTransformScale(self.pageView.transform,1.0,1.0);
        self.view.bounds = self.view.frame;
    } 

    if (orientation == UIInterfaceOrientationLandscapeLeft || orientation == UIInterfaceOrientationLandscapeRight) {        

        [self setNextAndPrevButtonsInLandscapeMode];
        toolImageView.frame = CGRectMake(410, 7, toolImageView.frame.size.width,  toolImageView.frame.size.height);


        if (editing) {
            pageView.bounds = editingLandscapePaperRect;
            bottomMenu.frame = editingLandscapeMenuRect;
        } else {
            bottomMenu.frame = defaultLandscapeMenuRect;
        }

        self.pageView.transform = CGAffineTransformIdentity;
        self.pageView.transform = CGAffineTransformMakeRotation(M_PI * (0) / 180.0);
        self.pageView.transform =   CGAffineTransformScale(self.pageView.transform,0.65,0.65);
        self.view.bounds = CGRectMake(-83.7, 73.7, 480.0f, 268.0f);
    } 

}

// Where most of the rotation works occurs
- (void)willAnimateRotationToInterfaceOrientation:(UIInterfaceOrientation)toInterfaceOrientation duration:(NSTimeInterval)duration
{
    [self handleOrientation:toInterfaceOrientation];
}

- (void)didRotateFromInterfaceOrientation:  (UIInterfaceOrientation)fromInterfaceOrientation
{

    // Resets next/prev buttons to portrait positions.
    if (self.interfaceOrientation == UIInterfaceOrientationPortrait ||  self.interfaceOrientation == UIInterfaceOrientationPortraitUpsideDown) {       
        previousButton.frame = CGRectMake(2, 2, 60, 60);
        nextButton.frame = CGRectMake(self.view.frame.size.width - 60 - 2, 2, 60, 60);
        undoButton.frame = CGRectMake(self.view.frame.size.width / 2 - 67, 2, 60, 60);
        redoButton.frame = CGRectMake(self.view.frame.size.width / 2 + 3, 2, 60, 60);      
        if (editing == NO) {
            [previousButton removeFromSuperview];
            [nextButton removeFromSuperview];
            [undoButton removeFromSuperview];
            [redoButton removeFromSuperview];
        }
    }
}

Here is the main view that relates to this view controller:

#import "PageView.h"
@implementation PageView
@synthesize paperView;
@synthesize canvas;

- (id)initWithFrame:(CGRect)frame
{
    self = [super initWithFrame:frame];
    if (self) {        
        if (paperView == nil) {
            paperView = [[PaperView alloc] initWithFrame:frame];
            [self addSubview:paperView];

            UIImage * ringImage = [UIImage imageNamed:@"BigNoteblockRings.png"];
            UIImageView * ringView = [[UIImageView alloc] initWithImage:ringImage];            
            CGRect ringRect = CGRectMake(- ringView.frame.size.width / 2 - 15, 5,   ringView.frame.size.width, ringView.frame.size.height);
            ringView.frame = ringRect;
            [paperView addSubview:ringView];            
        }

        if (canvas == nil) {
            /*
             Canvas is the area/context where the painting occurs.
             The image is accessed through UIImageView's .image method.
             */
            canvas = [[UIImageView alloc] init];
            canvas.frame = frame;
            [self addSubview:canvas];
        }        
    }
    return self;
}

Any tips on how to improve my code and my app's performance would be greatly appreciated.

\$\endgroup\$
3
\$\begingroup\$

First, there's just a couple of nit-picky things I'll comment on.


First, I'm quite sure this is totally unnecessary:

- (BOOL)shouldAutorotateToInterfaceOrientation (UIInterfaceOrientation)interfaceOrientation {
    return YES;
}   

I believe that this defaults to whatever orientations your app supports (and if a particular orientation isn't supported for your app, it will never try rotating to that orientation). The only reason to override this method is if you want to limit the rotation to fewer orientation options then the total list your app supports, so you should be able to simply completely eliminate these few lines of code from your view controller.


if (paperView == nil)

Can be replaced with simply if (!paperView). You have several instances of this. This wouldn't necessarily bother me so much, except that in the line right before it, you have:

if (self)

Which is effectively doing the same thing, just opposite. You're checking to see whether or not it's nil. If you think that if (self) is clear enough to say "I'm making sure self isn't nil," then if (!someObject) should be equally clear to the same reader that you're checking to make sure someObject is nil.


We have the same inconsistency with BOOL variables in another section of code.

In one line, we have:

if (editing)

And in another, we have

if (editing == NO)

And in this case, where we're dealing with a BOOL, I'm going to say we should definitely be using if (editing) and if (!editing) rather than the == comparison to NO (or worse, to YES).


@synthesize paperView;
@synthesize canvas;

I know this question was posted in December of 2011, but now, 30 months later, Xcode auto-synthesizes properties, so lines like this are completely unnecessary.


CGRect ringRect = CGRectMake(- ringView.frame.size.width / 2 - 15, 5,   ringView.frame.size.width, ringView.frame.size.height);

Likes like this are bothersome for two reasons. First of all, if the size isn't quite right, I have to sort out what part of this line is calculating each dimension. And second of all, the line is quite long. So there's two things we can do to fix it.

First, let's calculate each dimension separately.

CGFloat xPosition = - ringView.frame.size.width / 2 - 15;
CGFloat yPosition = 5;
CGFloat width = ringView.frame.size.width;
CGFloat height = ringView.frame.size.height;

And now, let's separate each argument on to separate lines if the single line is too long:

CGRect ringRect = CGRectMake(xPosition,
                             yPosition,
                             width,
                             height);

Xcode will take care of the alignment for you actually, all you have to do is hit the Enter key between each argument.


Now then, as for the performance and overall big picture?

Well, the thing is, I'd just replace everything with auto-layout. It doesn't look like you're doing anything fancy on rotation, just putting buttons in the right place. So, why not just use auto-layout?

From a performance point of view, auto-layout code is going to be constantly maintained, updated, and improved by Apple's engineers.

From a cleanliness and readability standpoint, a handful of interface builder clicks eliminates basically all of the code you've posted here and is less prone to error.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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