Homework Done - Diana

[Diana71]

No description provided.

[tannerwelsh]

FYI you can use NSBundle -URLForResource:withExtension: (docs) to do this in fewer steps and without an explicit dependency on file paths:

    NSURL *soundUrl2 = [[NSBundle mainBundle] URLForResource:@"PhoneRinging" withExtension:@".mp3"];
    _audioPlayerPhoneRinging = [[AVAudioPlayer alloc] initWithContentsOfURL:soundUrl2 error:nil];

This way, you can clean up your audio files into a subdirectory (group) within your bundle, like /Sounds, so that they don't clutter up your project list:

[tannerwelsh]

@Diana71 - great work on this project. You definitely went above and beyond. 👍 💃

Things you did well:

• Separating concerns between controllers & models
• Using the delegate pattern to send messages between controllers
• Naming variables, properties, and classes appropriately

I don't have that many suggestions for improvement, which is why I didn't add too many inline comments.

However, there is one area where your code could be improved, because it breaks proper encapsulation principles.

When you store all of the references to audio files w/in DetailsTableViewController (here), you are creating an explicit dependency between the audio files listed there and the contents of your options property of the current category.

This is not great because it means that your view controller now needs to know a lot about the details of the model, and you have lots of logic there which relies on this dependency.

To improve this, consider that the audio file is a form of data (i.e. state), and thus belongs to the model. It is still the controller's responsibility to actually play the audio file, but the model is responsible for allowing access to the audio file, or even exposing its own - play method.

Here's one way you could restructure your code to separate concerns more appropriately between your controller and model:

• Create a - playSoundForSelection method for CQCategory
• This method should be responsible for finding the MP3 file for the currently selected option (if it exists) and playing it
• Don't play any sounds within the tableView:cellForRowAtIndexPath: method of DetailsTableViewController (because this will play the file for each cell in the table view, which is redundant)
• Instead, send the playSoundForSelection message to the category whenever a row is selected (since this is the critical event that will play a new sound)

Here's an example implementation of the CQCategory using the new method:

#import "CQCategory.h"

@interface CQCategory ()
{
    AVAudioPlayer *_audioPlayer;
}
@end

@implementation CQCategory

- (void)playSoundForSelection {
    NSString *audioFileName = [self.selection stringByReplacingOccurrencesOfString:@" " withString:@""];
    NSURL *audioFileURL = [[NSBundle mainBundle] URLForResource:audioFileName withExtension:@".mp3"];

    if (audioFileURL == nil) {
        return; // If there isn't an audio file to be found, don't try to play it
    }

    _audioPlayer = [[AVAudioPlayer alloc] initWithContentsOfURL:audioFileURL error:nil];

    [_audioPlayer play];
}

@end

That way, all you need to do in DetailsTableViewController is call [self.category playSoundForSelection] when a row is selected, and you can rip out all of the code that manages different sound files for specific options.

The last piece that needs to be fixed is the sound you play when segueing to the DetailsTableViewController, and that really should be separate from the other sounds anyway. Just because they are both sound-related doesn't mean that they should be in the same part of your codebase.

I'd suggest adding a new method to your AllCategoriesTableViewController that plays the ButtonClickOn sound during prepareForSegue or add a method in DetailsTableViewController and call it from viewDidLoad.