# ViewController.h
First, this name isn't really descriptive at all, is it? We need to know more about this view controller. In fact, standard naming convention is to suffix all view controller subclasses with `ViewController` anyway, so this is basically an unnamed view controller.
Now, basically everything declared in your `.h` should be in the `.m` file. Of the 6 property declarations, 5 of them are `IBOutlet` properties, and there's never really an excuse for a view controller to expose its subviews publicly. A view controller should be in complete controller of its view. If an outside class needs to do something that would effect one of these views, create a public method, and let that method internally handle the effects on those views.
As for the 4 `IBAction` method declarations, you don't even need to declare these. The only reason to put a method in an `@interface` section is to expose it to another class. And `IBAction` methods should never be exposed to another class for the same point as exposing `IBOutlets` to outside classes. If you need to, create a public method and have that method call the `IBAction` method internally.
Finally, the `MBFQuotes` object. Does this need public visibility? Are you modifying it from outside classes? Looks like probably not, and as such, we can get rid of the class declaration (`@class MBFQuotes`) and move the `@property` declaration in the the `@interface` section of the `.m`, which comes after the `#import` of that classes file, and eliminates the need for declaring the class.
----------
# ViewController.m
Seeing as we're not actually doing anything in `didReceiveMemoryWarning`, we can simply eliminate that entire method from this file. All we're doing is calling the `super` method, and simply leaving the entire method out handles that. I understand that `didReceiveMemoryWarning`'s stub is generated automatically by Xcode when you create a new `UIViewController` subclass, but you should still take it out unless you intend to do something with it (and in most cases, you won't need this method).
Our four `IBAction` methods look quite similar, don't they? Why don't we [combine them into one method][1].
First, we need an `enum` for some readability, so at the top of this file, right after the `#import` statements, you'll want this code:
typedef NS_ENUM(NSInteger, QuoteTypes) {
GeneralQuotes = 100,
LifeQuotes = 101,
PoliticalQuotes = 102,
SmartQuotes = 103
};
Now, let's use the `.tag` property of `UIButton`. You can set this on the storyboard or programmatically. If you set it on the storyboard, set the tag for each button to match the appropriate number in the `enum` we just created. Programmatically, you can set them as such:
self.generalButton.tag = GeneralQuotes;
self.lifeButton.tag = LifeQuotes;
self.politicalButton.tag = PoliticalQuotes;
self.smartButton.tag = SmartQuotes;
Now then, get rid of the 4 `IBAction` methods (be sure to unlink the buttons in storyboard), and create on method to handle all 4 buttons, link all of the buttons to that method. The method should look something like this:
- (IBAction)quoteButtonPushed:(id)sender {
switch(sender.tag) {
case GeneralQuotes:
self.quoteLabel.text = [self.quotes randomGeneral];
self.quoteLabel.backgroundColor = [UIColor redColor];
break;
case LifeQuotes:
self.quoteLabel.text = [self.quotes randomLife];
self.quoteLabel.backgroundColor = [UIColor blueColor];
break;
case PoliticalQuotes:
self.quoteLabel.text = [self.quotes randomPolitical];
self.quoteLabel.backgroundColor = [UIColor blackColor];
break;
case SmartQuotes:
self.quoteLabel.text = [self.quotes randomSmart];
self.quoteLabel.backgroundColor = [UIColor orangeColor];
break;
default: break;
}
}
And please notice that I've used `self.quotes` here rather than `_quotes` as you used. If you're declaring a `@property`, you should be accessing that property through the accessor except in `init`, `dealloc`, and the accessor methods, should you choose to write custom accessor methods. If you don't want to access the property through the accessors, then maybe you don't need a property and instead just need a regular instance variable.
----------
# MBFQuotes.h / MBFQuotes.m
As it stands, this is really incomplete as a class, and doesn't really justify being a class at all. It could easily be simply a set of C-style functions. Which way do you want to go? Right now, you're in between.
If we're happy with the methods as they are and don't want to add anything, then let's change this to some C-style functions:
// MBFQuotes.h
#import <Foundation/Foundation.h>
NSString * randomGeneral();
NSString * randomLife();
NSString * randomPolitical();
NSString * randomSmart();
Notice the slightly different style and the elimination of `@interface` and `@end`. This is no longer a class, but instead a header file with some C-style function declarations.
EDIT: All right, so you can't actually have `const` NSArray objects as far as I can tell. The answer has been edited to the next best solution:
// MBFQuotes.m
#import "MBFQuotes.h"
NSString * randomGeneral() {
static NSArray *generalQuotes = @[@"g1", @"g2", @"g3", @"g4"];
int randomGeneral = arc4random_uniform([generalQuotes count]);
return [generalQuotes objectAtIndex:randomGeneral];
}
NSString * randomLife() {
static NSArray *lifeQuotes = @[@"l1", @"l2"];
int randomLife = arc4random_uniform([lifeQuotes count]);
return [lifeQuotes objectAtIndex:randomLife];
}
NSString * randomPolitical() {
static NSArray *politicsQuotes = @[@"p1", @"p2"];
int randomPolitical = arc4random_uniform([politicsQuotes count]);
return [politicsQuotes objectAtIndex:randomPolitical];
}
NSString * randomSmart() {
static NSArray *smartQuotes = @[@"s1", @"s2"];
int randomSmart = arc4random_uniform([smartQuotes count]);
return [smartQuotes objectAtIndex:randomSmart];
}
Notice moving the array declaration out of the functions/methods and declaring them as constants. This makes the program a lot more efficient.
Anyway, if you don't want to go this route, there is certainly room to turn this into a class. For starters, you'd still want to move the arrays out of the methods, but now you'll want them as private properties. For this to be truly classworthy though, you'd want to include methods to be able to add and remove quotes, perhaps give the class a file-path and parse quotes out of a text file.
Realistically, since this is a class and we're instantiating it, we shouldn't have different categories of quotes. Realistically, we should instantiate a different object of this class for each quote category.
UNLESS... we were to add a method to pick a random quote from ALL categories (single method, random category, random quote).
----------
Finally, I think for the project to be really complete, instead of the `MBHQuotes` class simply holding an array of strings, completeness would say there should be a Quote class, which consists of a few properties. The main property is the quote itself. The other properties give some information about the quote, such as the author, the date of the quote's origin, the category of quote it fits into, etc.
[1]: https://codereview.stackexchange.com/a/41762/36366