10
\$\begingroup\$

My task is pretty simple and my code works, it's just that my code feels a bit clumsy to me (and probably will for you too).

I have a view controller (HomeViewController) with a scrollView, inside this scroll view I have a label, and the scroll view is set to scroll horizontally. So now we have a view controller with a label and you can scroll this label.

What I want to do here is, whenever I scroll down the label a V sign icon will come from top, and whenever I the scroll view property contentOffset.y <= -73 I want to change the V sign icon to a different V sign icon.

I have some more parameters for changing this label. For instance, if there is even a label, because I take this label from a table view of another view controller and I can delete objects from this table view, and if there are no objects in this table view there is no label, in this case I don't want to present those icons when scrolling (this is also something I did in a pretty clumsy way, I think the best will be to disable the scroll view in this case).

This is my code (the relevant code, I'm working with nib's just so you know):

HomeViewController.m:

#import "HomeViewController.h"
//there are some more classes I'm importing here

@interface HomeViewController () <StackTableViewControllerDelegate, UIScrollViewDelegate>

@property (weak, nonatomic) IBOutlet UIImageView *currentScrollImage; //size 50X50
@property (weak, nonatomic) IBOutlet UIImageView *vMarkIcn; //size 28X20
@property (weak, nonatomic) IBOutlet UIImageView *greenVmarkIcn; //size 50X50
@property (assign, nonatomic) BOOL labelRemoving;

@end

@implementation HomeViewController {

       int fingerLiftedAtOffset;
}

//stackTableViewController is the table view where im taking the strings to populate the label

- (id)init {

    self = [super initWithNibName:@"HomeViewController" bundle:nil];
    if (self) {
        // Do something
        stackTableViewController = [[StackTableViewController alloc] init];
        stackTableViewController.delegate = self;
    }
    return self;
}


- (void)viewDidLoad {

    [super viewDidLoad];
    // Do any additional setup after loading the view from its nib.
    self.homeLabel.text = stackTableViewController.currentTarget;
    self.myScrollView.delegate = self;

    if (![stackTableViewController isEmpty]) {
        self.currentScrollImage.image = self.vMarkIcn.image;
    }
}

//here is the action/mess/clumsiness :)

- (void)viewWillAppear:(BOOL)animated {

    [stackTableViewController fetchData];
    self.homeLabel.text = stackTableViewController.currentTarget;
    [self.myScrollView becomeFirstResponder];
}

// here the scroll view icn's are updated according to the offset
- (void)scrollViewDidScroll:(UIScrollView *)scrollView {

    //AudioServicesPlaySystemSound(soundEffect3);

    if (![stackTableViewController isEmpty]) {
        self.currentScrollImage.image = self.vMarkIcn.image;
    }

    if (self.myScrollView.contentOffset.y <= -73 && ![stackTableViewController isEmpty]) {

        self.currentScrollImage.image = self.greenVmarkIcn.image;
    }
    NSLog(@"%f", self.myScrollView.contentOffset.y);
}

- (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView {

    if (![stackTableViewController isEmpty] && fingerLiftedAtOffset <= -73) {

        if (![stackTableViewController isEmpty]) {
            self.currentScrollImage.image = self.vMarkIcn.image;
        }

        [stackTableViewController removeObjectFromStack];
        if (![stackTableViewController isEmpty]) {
            AudioServicesPlaySystemSound(soundEffect2);
        }

    } else {
        self.currentScrollImage.image = nil;
    }
}

- (void)scrollViewDidEndDragging:(UIScrollView *)scrollView willDecelerate:(BOOL)decelerate {

    NSLog(@"finger was lifted");
    fingerLiftedAtOffset = self.myScrollView.contentOffset.y;
    if (self.myScrollView.contentOffset.y <= -73 && !self.labelRemoving && ![stackTableViewController isEmpty]) {

        [UIView animateWithDuration:1.5
                         animations:^{

                        self.homeLabel.frame = self.view.frame;
        }];
    }
}

- (void)viewWillAppear:(BOOL)animated {

    [stackTableViewController fetchData];
    self.homeLabel.text = stackTableViewController.currentTarget;
    [self.myScrollView becomeFirstResponder];
}

// here the scroll view icn's are updated according to the offset
- (void)scrollViewDidScroll:(UIScrollView *)scrollView {

    //AudioServicesPlaySystemSound(soundEffect3);

    if (![stackTableViewController isEmpty]) {
        self.currentScrollImage.image = self.vMarkIcn.image;
    }

    if (self.myScrollView.contentOffset.y <= -73 && ![stackTableViewController isEmpty]) {

        self.currentScrollImage.image = self.greenVmarkIcn.image;
    }
    NSLog(@"%f", self.myScrollView.contentOffset.y);
}

// here i'm removing the label from the stack
- (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView {

    if (![stackTableViewController isEmpty] && fingerLiftedAtOffset <= -73) {

        if (![stackTableViewController isEmpty]) {
            self.currentScrollImage.image = self.vMarkIcn.image;
        }

        [stackTableViewController removeObjectFromStack];
        if (![stackTableViewController isEmpty]) {
            AudioServicesPlaySystemSound(soundEffect2);
        }

    } else {
        self.currentScrollImage.image = nil;
    }
}

// this is a method to add animation when the finger is lifted so the new label will come from top
- (void)scrollViewDidEndDragging:(UIScrollView *)scrollView willDecelerate:(BOOL)decelerate {
    NSLog(@"finger was lifted");
    fingerLiftedAtOffset = self.myScrollView.contentOffset.y;
    if (self.myScrollView.contentOffset.y <= -73 && !self.labelRemoving && ![stackTableViewController isEmpty]) {

        [UIView animateWithDuration:1.5
                         animations:^{

                        self.homeLabel.frame = self.view.frame;
        }];
    }
}

StackTableViewController.m (only relevant code):

@interface StackTableViewController () <NSFetchedResultsControllerDelegate>

@end

@implementation StackTableViewController



- (id)init {

    self = [super initWithNibName:@"StackTableViewController" bundle:nil];
    if (self) {

        [self fetchData];
    }
    return self;
}

- (void)fetchData {
    [self.fetchedResultController performFetch:nil];
    if (self.fetchedResultController.fetchedObjects.count > 0) {
        NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
        Target *current = [self.fetchedResultController objectAtIndexPath:indexPath];
        self.currentTarget = current.body;
    }else {
        self.currentTarget = nil;
    }
}

- (BOOL)isEmpty {

    if (self.fetchedResultController.fetchedObjects.count > 0) {
        return NO;
    } else {
        return YES;
    }


- (void)removeObjectFromStack {

    if (self.fetchedResultController.fetchedObjects.count > 0) {

        NSIndexPath *indexPath = [NSIndexPath indexPathForItem:0 inSection:0];
        [self tableView:self.tableView commitEditingStyle:UITableViewCellEditingStyleDelete forRowAtIndexPath:indexPath];
        [self.fetchedResultController performFetch:nil];
        [self fetchData];
    }
}


- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath {

    Target *target = [self.fetchedResultController objectAtIndexPath:indexPath];
    CoreDataStack *stack = [CoreDataStack defaultStack];
    [[stack managedObjectContext] deleteObject:target] ;
    [stack saveContext];

    if ([_delegate respondsToSelector:@selector(didDeleteObject)]) {
        [self fetchData];
        [_delegate didDeleteObject];
    }

}

HomeViewController nib file documents outline:

enter image description here

So, it works, but I guess the code is bad.

I guess I need to only use 1 UIImageView and set its image, but I don't know how to do it with resizing the images how I want to, and now the V sign icon looked stretched, although I made currentScrollImage to be scroll to fill.

Something else I did weird is that I added two image objects to my HomeViewControllernib file, and added them to the scroll view, and I set their initial position to x=146, y = -50, and set them to be hidden in the IB, so when I scroll they will come down and I can change the image accordingly.

\$\endgroup\$
6
\$\begingroup\$

None of these points really help with what you're primarily asking about, and by all means, you certainly shouldn't mark this answer as accepted as it doesn't address your primary concern, but there are a few things I do want to point out about your code.


  1. Unless you're trying to report an error or warning to an end user (read: someone who has downloaded this from the app store), you should wrap all of your NSLog() statements in #if DEBUG statements so that they're only printed in debug mode and not printed in your final release build:

    #if DEBUG
        NSLog(@"this is a debug log");
    #endif
    

  1. -73 is a magic number. This has no meaning to anyone, and it may be that you'll forget its meaning at some point as well. More importantly however, what if you decide that you want to use a different value? In how many locations would you have to be certain you changed the value? Instead, we should define a constant and use that:

    static CGFloat const kSomeDescriptiveVariableName = -73.0f // offset to change image
    

  1. Our class names should have some sort of prefix. There's no namespaces in Objective-C. If you use StackTableViewController, and then later implement a library that also has a StackTableViewController, you're going to have problems. Now, fortunately, any library that's worth implementing probably won't have classes without prefixes... but that really doesn't mean you get to get away with not using prefixes. Xcode will auto generate and prepend these prefixes for you anyway if you just give it a value to use.

  1. Finally, our abundance of literal strings to load our nibs is actually the same as the -73 magic number problem. It's far less likely that you'll completely change the filename of the nib file, but it's not completely unheard of. For my personal preference, I prefer having a single file with all of my nib's defined as constants:

    // NibNames.h
    
    #ifndef ProjectName_NibNames_h
    #define ProjectName_NibNames_h
    
    static NSString * const kNIB_HomeViewController = @"HomeViewController";
    static NSString * const kNIB_StackTableViewController = @"StackTableViewController";
    
    #endif
    

    This gives us the same advantage as defining -73 as a constant, we can use it multiple places, and if we ever need to change it, it's one change in one spot and then we're done. But there's an added bonus when dealing with string literal. It's very easy to misspell a string literal. For me personally, I find misspellings and typos more frequent when I'm coding because code-completion is so helpful it makes me lazy. When using string literals, we get no code completion. When we define constants like this, we get code completion. Just start typing kNIB_ and you can use the arrow keys or the mouse scroll wheel to cycle through all your defined nib file names you've defined.

\$\endgroup\$
  • \$\begingroup\$ thanks for the tips buddy! it means allot really. about the NSLog's , I'm only using them right now to understand when things happens :) it's helping me learn. but the other points you mentioned I will implement them...what is the 'k' before the constant description mean? I saw this also in apples code I think @nhgrif \$\endgroup\$ – nick shmick Jan 23 '15 at 13:29
  • \$\begingroup\$ It's sort of just a historical prefix for constants. Let's us differentiate from our other variables more easily. I would still implement #if DEBUG wrappers so you don't forget about your log statements. \$\endgroup\$ – nhgrif Jan 23 '15 at 14:28
  • \$\begingroup\$ How can i make this post bounce? I really need an answer for the other stuff i mentioned...os there a service i can even pay for something like this? \$\endgroup\$ – nick shmick Jan 23 '15 at 14:48

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.