Logic in a card matching game controller

Question

I have a card matching game with a model of:

• Card
• Deck
• PlayingCard
• PlayingCardDeck

And another model called CardMatchingGame.

Now, I had a task to add a button to the view for new game, which deals the cards again and setting the labels to 0 (#flips and score).

I added this method:

- (IBAction)newGame:(UIButton *)sender {

    self.flipsCount = 0;
    self.game = nil;
    for (UIButton *button in self.cardButtons) {
        Card *card = [self.game cardAtIndex:[self.cardButtons indexOfObject:button]];
        card.unplayble = NO;
        card.faceUp = NO;
        button.alpha = 1;
    }

    self.notificationLabel.text = nil;
    [self updateUI];
}

And this is the whole viewcontroller:

#import "CardGameViewController.h"
#import "PlayingCardsDeck.h"
#import "CardMatchingGame.h"


@interface CardGameViewController ()

@property (weak, nonatomic) IBOutlet UILabel *flipsLabel;
@property (weak, nonatomic) IBOutlet UILabel *notificationLabel;
@property (weak, nonatomic) IBOutlet UILabel *scoreCounter;
@property (strong, nonatomic) IBOutletCollection(UIButton) NSArray *cardButtons;

@property (strong, nonatomic) CardMatchingGame *game;
@property (nonatomic) int flipsCount;

@end


@implementation CardGameViewController

//creating the getter method that creates a new card game.
-(CardMatchingGame *) game {

    if (!_game) _game = [[CardMatchingGame alloc] initWithCardCount:self.cardButtons.count usingDeck:[[PlayingCardsDeck alloc] init]];
    return _game;
}

//creating a setter for the IBOutletCollection cardButtons
-(void) setCardButtons:(NSArray *)cardButtons {

    _cardButtons = cardButtons;
   [self updateUI];
}


//creating the setter for the flipCount property. Whick is setting the flipsLabel to the right text and adding the number of counts.
-(void) setFlipsCount:(int)flipsCount {

    _flipsCount = flipsCount;
    self.flipsLabel.text = [NSString stringWithFormat:@"Flips: %d", self.flipsCount];

}


-(void) updateUI {

    for (UIButton *cardButton in self.cardButtons) {
        Card *card = [self.game cardAtIndex:[self.cardButtons indexOfObject:cardButton]];
        [cardButton setTitle:card.contents forState:UIControlStateSelected];
        [cardButton setTitle:card.contents forState:UIControlStateSelected|UIControlStateDisabled];
        cardButton.selected = card.isFaceUp;
        cardButton.enabled = !card.unplayble;
        if (card.unplayble) {
            cardButton.alpha = 0.1;
        }

        //updating the score 
        self.scoreCounter.text = [NSString stringWithFormat:@"Score: %d", self.game.score];

        //if notification in CardMatchingGame.m is no nil, it will be presented 
        if (self.game.notification) {

        self.notificationLabel.text = self.game.notification;

        }
    }
}

//Here I created a method to flipCards when the card is selected, and give the user a random card from the deck each time he flips the card. After each flip i'm incrementing the flipCount setter by one.
- (IBAction)flipCard:(UIButton *)sender {

    [self.game flipCardAtIndex:[self.cardButtons indexOfObject:sender]];
    self.flipsCount++;
    [self updateUI];
}


//cleaning the history using the cleanHistory method in CardMatchingGame.m and creating a new game by setting geme to nil
- (IBAction)newGame:(UIButton *)sender {

    self.flipsCount = 0;
    self.game = nil;
    for (UIButton *button in self.cardButtons) {
        Card *card = [self.game cardAtIndex:[self.cardButtons indexOfObject:button]];
        card.unplayble = NO;
        card.faceUp = NO;
        button.alpha = 1;
    }

    self.notificationLabel.text = nil;
    [self updateUI];
}


@end

Could you please tell me if this is a logical solution? I'm a very new programmer and critiques are important to me.

Answer 1

You certainly get points for using IBOutletCollection(UIButton). But, it's unclear why you want to replace this array with a new one? If you're going to do that it should be this view controller that creates the new buttons and adds them as subviews. Also, if you're going to create the buttons in code then I wouldn't expect the first set to be defined in the XIB.

I'm assuming (based on your property definitions) that you're using ARC too.

This section of code can be better by maintaining and incrementing an index instead of interrogating the array you're looping over for the index:

for (UIButton *cardButton in self.cardButtons) {
    Card *card = [self.game cardAtIndex:[self.cardButtons indexOfObject:cardButton]];

Also, in the updateUI button loop you don't need to set the title multiple times to the same thing and the loop should be completed before you modify the label text.

You want to know about logic, but there is basically no logic in this class. What this class does have is a bit of confusion about whether it owns the buttons and associated cards or if some other class does. The only comments that can be made really relate to efficiency rather than logic.