3
\$\begingroup\$

I'm a beginner developer, I'm learning online (mostly from the Apple documentation guides), and couple a days ago I started a notes app project. It's the first app I'm building without a guide.

It's very simple so I'd love to post it here and get your feedback on how to improve my code to be more efficient.

The app have two view controllers:

  1. NotesListViewController

  2. CreateNotesViewController

I use Core Data so I created an entity called Note that have one string attribute called content, after that I created the model class from the entity section by doing editor/create NSManagedObject subclass.

So those are my class:

NotesListViewController.h:

#import <UIKit/UIKit.h>

@interface NotesListViewController : UITableViewController

- (IBAction) unwindToList:(UIStoryboardSegue *)segue;

@end

NotesListViewController.m:

#import "NotesListViewController.h"
#import "Note.h"
#import "CreateNotesViewController.h"

@interface NotesListViewController ()

@property (nonatomic, strong) NSMutableArray *notes;
@property (nonatomic) NSInteger editedRow;

@end


@implementation NotesListViewController

- (NSManagedObjectContext *) managedObjectContext
{
    NSManagedObjectContext *context = nil;

    id delegate = [[UIApplication sharedApplication] delegate];

    if ([delegate performSelector:@selector(managedObjectContext)]){
        context = [delegate managedObjectContext];
    }

    return context;
}


- (void) prepareForSegue:(UIStoryboardSegue *)segue sender:(id)sender
{
    if ([[segue identifier] isEqualToString:@"editSegue"]) {
        CreateNotesViewController *destination = (CreateNotesViewController *)[segue destinationViewController];
        NSInteger indx = [self.tableView indexPathForCell:sender].row;
        Note *noteToPass = self.notes[indx];
        destination.note = noteToPass;
        self.editedRow = indx;

        NSManagedObject *selectedNote = [self.notes objectAtIndex:[self.tableView indexPathForSelectedRow].row];
        destination.editNote = selectedNote;

    } else if ([[segue identifier] isEqualToString:@"addSegue"]) {
        self.editedRow = -1;
    }
}


- (IBAction)unwindToList:(UIStoryboardSegue *)segue
{
    CreateNotesViewController *source = (CreateNotesViewController *)[segue sourceViewController];
    Note *recieivedNote = source.note;

    if (recieivedNote != nil && self.editedRow == -1) {
        [self.notes addObject:recieivedNote];
    } else  if  (recieivedNote != nil)

    {
        [self.notes replaceObjectAtIndex:self.editedRow withObject:recieivedNote];
    }

    [self.tableView reloadData];
}


- (id)initWithStyle:(UITableViewStyle)style
{
    self = [super initWithStyle:style];
    if (self) {
        // Custom initialization
    }
    return self;
}


- (void)viewDidLoad
{
    [super viewDidLoad];
    self.notes = [[NSMutableArray alloc] init];
}


- (void) viewWillAppear:(BOOL)animated
{
    [super viewWillAppear:animated];

    NSManagedObjectContext *managedObjectContext = [self managedObjectContext];

    NSFetchRequest *fetchRequest = [[NSFetchRequest alloc] initWithEntityName:@"Note"];

    self.notes = [[managedObjectContext executeFetchRequest:fetchRequest error:nil] mutableCopy];

    [self.tableView reloadData];
}


- (void)didReceiveMemoryWarning
{
    [super didReceiveMemoryWarning];
    // Dispose of any resources that can be recreated.
}


#pragma mark - Table view data source


- (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView
{
    return 1;
}


- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section
{
    // Return the number of rows in the section.
    return self.notes.count;
}


- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath
{
    static NSString *CellIdentifier = @"Cell";
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:CellIdentifier forIndexPath:indexPath];

    // Configure the cell...

    NSManagedObject *noteToDisplay = [self.notes objectAtIndex:indexPath.row];

    [cell.textLabel setText:[noteToDisplay valueForKey:@"content"]];

    return cell;
}


- (BOOL) tableView:(UITableView *)tableView canEditRowAtIndexPath:(NSIndexPath *)indexPath
{
    return YES;
}


- (void)tableView:(UITableView *)tableView commitEditingStyle:(UITableViewCellEditingStyle)editingStyle forRowAtIndexPath:(NSIndexPath *)indexPath
{
    NSManagedObjectContext *context = [self managedObjectContext];

    if (editingStyle == UITableViewCellEditingStyleDelete)

    {
        [context deleteObject:[self.notes objectAtIndex:indexPath.row]];

        NSError *error = nil;
        if ([context save:&error]) {
            NSLog(@"Can't Delete! %@ %@", error, [error localizedDescription]);
        }
        [self.notes removeObjectAtIndex:indexPath.row];
        [self.tableView deleteRowsAtIndexPaths:[NSArray arrayWithObject:indexPath] withRowAnimation:UITableViewRowAnimationLeft];
    }

}


@end

CreateNotesViewController.h:

#import <UIKit/UIKit.h>
#import "Note.h"

@interface CreateNotesViewController : UIViewController

@property (nonatomic, strong) Note *note;
@property (strong) NSManagedObject *editNote;

@end

CreateNotesViewController.m:

#import "CreateNotesViewController.h"

@interface CreateNotesViewController ()

@property (weak, nonatomic) IBOutlet UIBarButtonItem *saveButton;
@property (weak, nonatomic) IBOutlet UITextView *myTextView;

@end


@implementation CreateNotesViewController

- (NSManagedObjectContext *) managedObjectContext
{
    NSManagedObjectContext *context = nil;

    id delegate = [[UIApplication sharedApplication] delegate];

    if ([delegate performSelector:@selector(managedObjectContext)]){
        context = [delegate managedObjectContext];
    }

    return context;
}


- (void) prepareForSegue:(UIStoryboardSegue *)segue sender:(id)sender
{
    if (sender != self.saveButton) return;
    if (self.myTextView.text.length > 0) {

        self.note.content = self.myTextView.text;

        NSManagedObjectContext *context = [self managedObjectContext];

        if (self.editNote) {
            [self.editNote setValue:self.myTextView.text forKey:@"content"];
        } else {

        // creating a new managed object
        NSManagedObject *newNote = [NSEntityDescription insertNewObjectForEntityForName:@"Note" inManagedObjectContext:context];
        [newNote setValue:self.myTextView.text forKey:@"content"];
        }

        NSError *error = nil;

        if (![context save:&error]) {
            NSLog(@"Can't Save! %@ %@", error, [error localizedDescription]);
        }
        else
        {
            NSLog(@"Saved! %@ %@", error, self.myTextView.text);
        }
    }
}


- (void)viewWillAppear:(BOOL)animated
{
    [super viewWillAppear:animated];

    self.myTextView.text = self.note.content;

    // listen for keyboard hide/show notifications so we can properly adjust the table's height
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(keyboardWillShow:)
                                                 name:UIKeyboardWillShowNotification
                                               object:nil];
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(keyboardWillHide:)
                                                 name:UIKeyboardWillHideNotification
                                               object:nil];

}


- (void)adjustViewForKeyboardReveal:(BOOL)showKeyboard notificationInfo:(NSDictionary *)notificationInfo
{
    // the keyboard is showing so ƒ the table's height
    CGRect keyboardRect = [[notificationInfo objectForKey:UIKeyboardFrameEndUserInfoKey] CGRectValue];
    NSTimeInterval animationDuration =
    [[notificationInfo objectForKey:UIKeyboardAnimationDurationUserInfoKey] doubleValue];
    CGRect frame = self.myTextView.frame;

    // the keyboard rect's width and height are reversed in landscape
    NSInteger adjustDelta = UIInterfaceOrientationIsPortrait(self.interfaceOrientation) ? CGRectGetHeight(keyboardRect) : CGRectGetWidth(keyboardRect);

    if (showKeyboard)
    frame.size.height -= adjustDelta;
    else
    frame.size.height += adjustDelta;

    [UIView beginAnimations:@"ResizeForKeyboard" context:nil];
    [UIView setAnimationDuration:animationDuration];
    self.myTextView.frame = frame;
    [UIView commitAnimations];
}


- (void)keyboardWillShow:(NSNotification *)aNotification
{
    [self adjustViewForKeyboardReveal:YES notificationInfo:[aNotification userInfo]];
}


- (void)keyboardWillHide:(NSNotification *)aNotification
{
    [self adjustViewForKeyboardReveal:NO notificationInfo:[aNotification userInfo]];
}


- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil
{
    self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self) {
        // Custom initialization
    }
    return self;
}


- (void)viewDidLoad
{
    [super viewDidLoad];
    // Do any additional setup after loading the view.
}


- (void)didReceiveMemoryWarning
{
    [super didReceiveMemoryWarning];
    // Dispose of any resources that can be recreated.
}

@end

Note.h:

#import <Foundation/Foundation.h>
#import <CoreData/CoreData.h>


@interface Note : NSManagedObject

@property (nonatomic, retain) NSString * content;

@end

In the Note.m file I didn't added anything:

#import "Note.h"


@implementation Note

@dynamic content;

@end

And I use storyboard to create my interface that looks like this:

enter image description here

So it's very basic, you can only:

  1. create a note (saved using core data)
  2. view your notes list
  3. edit a note (updated using core data)
  4. delete a note (deleted using core data)

I really enjoy programming in Objective-C and want to get better and better, so if you could be kind enough to give me your feedback on the code that would be awesome.

\$\endgroup\$
2
\$\begingroup\$

Look into the NSFetchedResultsController to power your table view. It's something specifically designed to be used with UITableViews that are powered by Core Data.

In essence, it uses a fetch request and returns how many objects are in each section, how many sections there are, what header/footer information they have etc. It has the added benefit that if the underlying managed object context changes (for example when a new object is added, updated or deleted) the fetched results controller is updated automatically. When that happens you can send your table view a reloadData message so it's updated as if by magic. An NSFetchedResultsController adds another level of complexity to Core Data, but since you're already confidently using the framework my guess is you'll like it ;-)

As a side note: look into Protocols. UIKit uses them extensively, they can be very useful and they're easy to create. With Protocols you can put one controller in charge of presenting and dismissing another view controller without having to rely on an unwind segue. The benefit is that one controller can hold on to a managed object, it gets changed in another view controller, and when that's dismissed the originating controller can use the changed object (to save or evaluate it).

I've got a full working project on GitHub which illustrates both the NSFetchedResultsController and uses a Protocol to dismiss the Detail View. It's similar to what you're building (a CRUD Project - as in Create, Read, Update, Delete):

https://github.com/versluis/Master-Detail-CRUD

Then of course there's adding a search feature to your table view... but that's for another time. When you're ready, I have a demo project for that too: https://github.com/versluis/TableSearch2014

Let me know if it helps, and have fun with Objective C!

PS: Kudos for sharing, and for not starting from a template. Well done!

\$\endgroup\$
  • \$\begingroup\$ It helps allot :) thank you. I will implement your suggestions! @Jay Versluis \$\endgroup\$ – Nir Apr 19 '14 at 23:21
2
\$\begingroup\$

I don't know a whole lot about CoreData or NSManagedObjects, so I can't offer a whole lot of help in this realm, but here are some general style things I noticed (plus a few other things).

You don't need the unwind segue method in your .h file. You can still hook up storyboard segues with it only being in the @implementation, and no other class needs to call this method.

I can't see when the NSManagedContext property, editNote is used outside the second view controller, so this likewise probably doesn't need to be in the .h (but maybe I'm overlooking something).


id delegate = [[UIApplication sharedApplication] delegate];

The return from this method call is an object of type UIApplication. Your delegate variable should also be of type UIApplication, rather than simply id.

Moreover, what you do in the following line doesn't make sense. I think you're trying to do a verification check that the delegate object responds to that selector, in which case you want:

if ([delegate respondsToSelector:@selector(managedObjectContext)]){
    context = [delegate managedObjectContext];
}

When preparing for a segue and doing anything with a destination view controller, it's not good enough to simply compare the name of the destination segue. We should also be verifying the class of the destination view controller. In such a simple project, you won't run into a problem, and personally, I make the habit of giving every segue a unique name, but this isn't always necessarily the case. The proper way to grab a destination view controller looks like this:

if ([[segue identifier] isEqualToString:@"MySegue]) {
    if ([[segue destinationViewController] isKindOfClass:[MyViewController class]]) {
        MyViewController *destination = [segue destinationViewController];
    }
}

I can't understand what or why you're doing what you're doing in CreateNotesViewController's prepareForSegue. We need to do this stuff before we try to segue, given the possibility of a save failure.

The save button should have an IBAction method of its own. It should go through the save process. If you want it to immediately segue after saving, then have it call the segue programmatically. But certainly after a save failure, perhaps the user should get the choice to either segue (and lose all their unsaved stuff, depending on segue type and other stuff) or just segue anyway.

And either way, this sorts of code doesn't belong in prepareForSegue--that method is designed to allow you to set up the destination view controller and pass information to it--you shouldn't be slowing down a segue with something like a save process.


Methods like this:

- (id)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibBundleOrNil
{
    self = [super initWithNibName:nibNameOrNil bundle:nibBundleOrNil];
    if (self) {
        // Custom initialization
    }
    return self;
}


- (void)viewDidLoad
{
    [super viewDidLoad];
    // Do any additional setup after loading the view.
}


- (void)didReceiveMemoryWarning
{
    [super didReceiveMemoryWarning];
    // Dispose of any resources that can be recreated.
}

Can and should be simply deleted. This is a subclass. If you're doing nothing with the method beyond invoking the superclass's implementation, then it's nothing but clutter--especially for one of the subclassed objects in all of iOS.


Right now I don't understand the need for the Note class. Why doesn't an NSString work instead of a class with no methods and a single NSString property?

\$\endgroup\$
  • \$\begingroup\$ The Note class is used by Core Data, it's a Managed Object Subclass. \$\endgroup\$ – Jay Versluis May 1 '14 at 11:11
  • \$\begingroup\$ Ah, I see that now. Makes more sense. \$\endgroup\$ – nhgrif May 1 '14 at 11:26

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.