Gfrktyl

I am building a Quiz app in swift (my first app) to practice a few skills (using CoreData, working with plists, UIKit etc.) and later to work with a server which stores the Exercises.

Here's my current point in development: https://bitbucket.org/paescebu/staplerch/src/master/

My question is not precise, more general, I hope this is accepted here. I wanted to know if what I am practicing in my Code is good or bad practice so far.
Any specifics like:
- Design pattern
- Conventions
- bad practices
are welcome

example of my uncertainties (my AppDelegate):

import UIKit

import CoreData



@UIApplicationMain

class AppDelegate: UIResponder, UIApplicationDelegate {



var window: UIWindow?



func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {

    // Override point for customization after application launch.

    preloadData()

    return true

}



private func preloadData() {



    let preloadedDataKey = "didPreloadData"

    let userDefaults = UserDefaults.standard

    if userDefaults.bool(forKey: preloadedDataKey) == false

    {

        var categories: [Category] = 

        if let path = Bundle.main.path(forResource: "demoExercises", ofType: "plist") {

            categories = readDemoDataPlist(withPath: path)

        }

        //preload into Core Data

        let backgroundContext = persistentContainer.newBackgroundContext()

        persistentContainer.viewContext.automaticallyMergesChangesFromParent = true

        backgroundContext.perform

            {

            do {

                for item in categories {

                    let category = CDCategory(context: backgroundContext)

                    category.title = item.title

                    category.imageName = item.imageName

                    if let exerciseArray = item.exercises {

                        for eachExercise in exerciseArray {

                            let exercise = CDExercise(context: backgroundContext)

                            exercise.question = eachExercise.question

                            exercise.explanation = eachExercise.explanation

                            exercise.imageName = eachExercise.imageName

                            exercise.correctAnswer = eachExercise.correctAnswer

                            if let answersArray = eachExercise.answers

                            {

                                for eachAnswer in answersArray {

                                    let answer = CDAnswers(context: backgroundContext)

                                    answer.answer = eachAnswer

                                    exercise.addToAnswers(answer)

                                }

                            }

                            category.addToExercises(exercise)

                        }

                    }

                }

                try backgroundContext.save()

                userDefaults.set(true, forKey: preloadedDataKey)

                print("Sucess!")

            }

            catch {

                print("failed saving Context:(error.localizedDescription)")

            }



        }

    }

}



func readDemoDataPlist(withPath: String) -> [Category] {

    var categoriesArray : [Category] = 

    if let arrayWithCategories = NSArray(contentsOfFile: withPath) as? [[String : Any]] {

        for eachCategory in arrayWithCategories {

            let category = Category()

            if let categoryTitle = eachCategory["title"] as? String {

                category.title = categoryTitle

            }

            if let categoryImage = eachCategory["imageName"] as? String {

                category.imageName = categoryImage

            }

            if let arrayWithExercises = eachCategory["exercises"] as? [[String : Any]] {

                var exerciseArray: [Exercise] = 

                for eachExercise in arrayWithExercises {

                    let exercise = Exercise()

                    if let question = eachExercise["question"] as? String {

                        exercise.question = question

                    }

                    if let correctAnswerIndex = eachExercise["correctAnswer"] as? String {

                        exercise.correctAnswer = correctAnswerIndex

                    }

                    if let answers = eachExercise["answers"] as? [String] {

                        exercise.answers = answers

                    }

                    if let image = eachExercise["image"] as? String {

                        exercise.imageName = image

                    }

                    if let explanation = eachExercise["explanation"] as? String {

                        exercise.explanation = explanation

                    }

                    exerciseArray.append(exercise)

                    category.exercises = exerciseArray

                }

            }

            categoriesArray.append(category)

        }

    }

    return categoriesArray

}



func applicationWillResignActive(_ application: UIApplication) {

    // Sent when the application is about to move from active to inactive state. This can occur for certain types of temporary interruptions (such as an incoming phone call or SMS message) or when the user quits the application and it begins the transition to the background state.

    // Use this method to pause ongoing tasks, disable timers, and invalidate graphics rendering callbacks. Games should use this method to pause the game.

}



func applicationDidEnterBackground(_ application: UIApplication) {

    // Use this method to release shared resources, save user data, invalidate timers, and store enough application state information to restore your application to its current state in case it is terminated later.

    // If your application supports background execution, this method is called instead of applicationWillTerminate: when the user quits.

}



func applicationWillEnterForeground(_ application: UIApplication) {

    // Called as part of the transition from the background to the active state; here you can undo many of the changes made on entering the background.

}



func applicationDidBecomeActive(_ application: UIApplication) {

    // Restart any tasks that were paused (or not yet started) while the application was inactive. If the application was previously in the background, optionally refresh the user interface.

}



func applicationWillTerminate(_ application: UIApplication) {

    // Called when the application is about to terminate. Save data if appropriate. See also applicationDidEnterBackground:.

    // Saves changes in the application's managed object context before the application terminates.

    self.saveContext()

}



// MARK: - Core Data stack



lazy var persistentContainer: NSPersistentContainer = {

    /*

     The persistent container for the application. This implementation

     creates and returns a container, having loaded the store for the

     application to it. This property is optional since there are legitimate

     error conditions that could cause the creation of the store to fail.

    */

    let container = NSPersistentContainer(name: "StaplerCH")

    container.loadPersistentStores(completionHandler: { (storeDescription, error) in

        if let error = error as NSError? {

            // Replace this implementation with code to handle the error appropriately.

            // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development.



            /*

             Typical reasons for an error here include:

             * The parent directory does not exist, cannot be created, or disallows writing.

             * The persistent store is not accessible, due to permissions or data protection when the device is locked.

             * The device is out of space.

             * The store could not be migrated to the current model version.

             Check the error message to determine what the actual problem was.

             */

            fatalError("Unresolved error (error), (error.userInfo)")

        }

    })

    return container

}()



// MARK: - Core Data Saving support



func saveContext () {

    let context = persistentContainer.viewContext

    if context.hasChanges {

        do {

            try context.save()

        } catch {

            // Replace this implementation with code to handle the error appropriately.

            // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development.

            let nserror = error as NSError

            fatalError("Unresolved error (nserror), (nserror.userInfo)")

            }

        }

    }  

}

UPDATE

I changed it quite a bit now, directly using the NSManagedObject model created from CoreData, instead of this step in between storing from plist, and the reading into persistent store.

Looks now like this:

import UIKit

import CoreData



@UIApplicationMain

class AppDelegate: UIResponder, UIApplicationDelegate {

var window: UIWindow?



func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {

    // Override point for customization after application launch.

    preloadData()

    return true

}



private func preloadData() {



    let preloadedDataKey = "didPreloadData"

    let userDefaults = UserDefaults.standard

    if userDefaults.bool(forKey: preloadedDataKey) == false

    {

        if let path = Bundle.main.path(forResource: "demoExercises", ofType: "plist")

        {

            if let arrayWithCategories = NSArray(contentsOfFile: path) as? [[String : Any]]

            {

                //preload into Core Data

                let backgroundContext = persistentContainer.newBackgroundContext()

                persistentContainer.viewContext.automaticallyMergesChangesFromParent = true

                backgroundContext.perform {

                    do {

                        for eachCategory in arrayWithCategories {

                            let category = Category(context: backgroundContext)

                            if let categoryTitle = eachCategory["title"] as? String {

                                category.title = categoryTitle

                            }

                            if let categoryImage = eachCategory["imageName"] as? String {

                                category.imageName = categoryImage

                            }

                            if let exerciseArray = eachCategory["exercises"] as? [[String: Any]] {

                                for eachExercise in exerciseArray {

                                    let exercise = Exercise(context: backgroundContext)

                                    if let question = eachExercise["question"] as? String {

                                        exercise.question = question

                                    }

                                    if let image = eachExercise["image"] as? String {

                                        exercise.imageName = image

                                    }

                                    if let explanation = eachExercise["explanation"] as? String {

                                        exercise.explanation = explanation

                                    }

                                    if let arrayWithAnswers = eachExercise["answers"] as? [[String : Any]] {

                                        for eachAnswer in arrayWithAnswers {

                                            if let answerText = eachAnswer["text"] as? String, let answerIsCorrect = eachAnswer["isCorrect"] as? Bool {

                                                let answer = Answer(context: backgroundContext)

                                                answer.text = answerText

                                                answer.isCorrect = answerIsCorrect

                                                exercise.addToAnswers(answer)

                                            }

                                        }

                                    }

                                    category.addToExercises(exercise)

                                }

                            }

                        }

                        try backgroundContext.save()

                        userDefaults.set(true, forKey: preloadedDataKey)

                        print("Sucess!")

                    }

                    catch {

                        print("failed saving Context:(error.localizedDescription)")

                    }



                }

            }

        }

    }

}



func applicationWillResignActive(_ application: UIApplication) {

    // Sent when the application is about to move from active to inactive state. This can occur for certain types of temporary interruptions (such as an incoming phone call or SMS message) or when the user quits the application and it begins the transition to the background state.

    // Use this method to pause ongoing tasks, disable timers, and invalidate graphics rendering callbacks. Games should use this method to pause the game.

}



func applicationDidEnterBackground(_ application: UIApplication) {

    // Use this method to release shared resources, save user data, invalidate timers, and store enough application state information to restore your application to its current state in case it is terminated later.

    // If your application supports background execution, this method is called instead of applicationWillTerminate: when the user quits.

}



func applicationWillEnterForeground(_ application: UIApplication) {

    // Called as part of the transition from the background to the active state; here you can undo many of the changes made on entering the background.

}



func applicationDidBecomeActive(_ application: UIApplication) {

    // Restart any tasks that were paused (or not yet started) while the application was inactive. If the application was previously in the background, optionally refresh the user interface.

}



func applicationWillTerminate(_ application: UIApplication) {

    // Called when the application is about to terminate. Save data if appropriate. See also applicationDidEnterBackground:.

    // Saves changes in the application's managed object context before the application terminates.

    self.saveContext()

}



// MARK: - Core Data stack



lazy var persistentContainer: NSPersistentContainer = {

    /*

     The persistent container for the application. This implementation

     creates and returns a container, having loaded the store for the

     application to it. This property is optional since there are legitimate

     error conditions that could cause the creation of the store to fail.

     */

    let container = NSPersistentContainer(name: "StaplerCH")

    container.loadPersistentStores(completionHandler: { (storeDescription, error) in

        if let error = error as NSError? {

            // Replace this implementation with code to handle the error appropriately.

            // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development.



            /*

             Typical reasons for an error here include:

             * The parent directory does not exist, cannot be created, or disallows writing.

             * The persistent store is not accessible, due to permissions or data protection when the device is locked.

             * The device is out of space.

             * The store could not be migrated to the current model version.

             Check the error message to determine what the actual problem was.

             */

            fatalError("Unresolved error (error), (error.userInfo)")

        }

    })

    return container

}()



// MARK: - Core Data Saving support



func saveContext () {

    let context = persistentContainer.viewContext

    if context.hasChanges {

        do {

            try context.save()

        } catch {

            // Replace this implementation with code to handle the error appropriately.

            // fatalError() causes the application to generate a crash log and terminate. You should not use this function in a shipping application, although it may be useful during development.

            let nserror = error as NSError

            fatalError("Unresolved error (nserror), (nserror.userInfo)")

        }

    }

}    

}

Short workflow: After first installation, the app will load some demoQuestions from a plist into the persistent Store using the CoreData stack, so that the app is already preloaded with some Exercises, as said, in a later stage I'd like to be able to keep Exercises on a server from which the user can keep the Exercises updated.

The App has 2 Modes, a Practice and an Exam Mode (depending of the mode the ExerciseVC behaves differently), I really tried hard to have one VC only for that.

The other two Pages of the Start Page are quite irrelevant so far.

I am very thankful for any input.

Coding style

Generally your code is written clearly. Just two points that I noticed:

• 
  

  Two different styles of positioning the opening braces of a code block are used:

  
  
  

  if userDefaults.bool(forKey: preloadedDataKey) == false
  
  {
  
  

  
  
  

  vs

  
  
  

  if let exerciseArray = item.exercises {
  
  

  
  
  

  I prefer the second version, that is also what you'll find in the “Swift Programming Language” book and in the Swift source code. You can choose your favorite style, but it should be used consistently.

  
  


• 
  

  Testing boolean values: I prefer

  
  
  

  if !someBooleanExpression 
  
  

  
  
  

  over

  
  
  

  if someBooleanExpression == false
  
  

  
  

Naming variables

From the Swift API Design Guidelines,

Name variables, parameters, and associated types according to their roles, rather than their type constraints.

This applies to exerciseArray, answersArray etc in your code. Better choices would be exercises, answers. I would also omit the each prefix in eachExercise, eachAnswer, just

for exercise in exercises { ... }

In your first approach, where you have “plain” model classes in addition to the Core Data classes you could use the “cd” prefix to distinguish between the variables – as you already do to distinguish the classes:

for exercise in exercises {

    let cdexercise = CDExercise(context: backgroundContext)

    ceexercise.question = exercise.question

    // ...

}

The Core Data models

It seems that most (all?) attributes in the Core Data model classes are declared as optionals. But what happens if an attribute is not set? An exercise without question, an answer without text? It could lead to a runtime error later, or to unexpected results. You should check which attributes are required, and declare those as non-optionals.

The isCorrect attribute of CDAnswer seems unnecessary since there is already a correctAnswer attribute in CDExercise. However, I would make that attribute a relationship to CDAnswer instead of a string.

The property list

From

if let categoryImage = eachCategory["imageName"] as? String

// ...

if let image = eachExercise["image"] as? String {

it seems that the keys in the default property list are not consistent. I'd suggest to use same names as the properties in the model classes, to avoid confusion.

To force unwrap or not to force unwrap – loading resources

You carefully use optional binding and conditional casts, which is generally a good practice to avoid runtime errors. However, when loading the property list, you know that it is present, and what it contains. Any error here would be a programming error and must be fixed before deploying the program.

Therefore forced unwrapping/casting is actually better: It helps to detect the programming error early:

let url = Bundle.main.url(forResource: "demoExercises", withExtension: "plist")!

// ...

The same applies to the optional casts: You know that category has a key "exercises" with a value which is an array of dictionaries, so instead of

if let arrayWithExercises = eachCategory["exercises"] as? [[String : Any]]

you can write

let arrayWithExercises = eachCategory["exercises"] as! [[String : Any]]

Approach #1: Loading the data into non-Core Data model objects first

Your first approach is to load the default data into separate structures first, and then create the managed objects. That requires some extra space and time, but that matters only if the default data is big.

It can be simplified considerably however, using a PropertyListDecoder. It suffices to declare conformance to the Decodable protocol in your plain model classes

struct Category: Decodable {

    let title: String

    let imageName: String

    let exercises: [Exercise]

}



struct Exercise: Decodable { ... }



struct Answer: Decodable { ... }

and to make sure that the property names are identical to the keys in the property list file. Then reading the default data becomes as simple as

let url = Bundle.main.url(forResource: "demoExercises", withExtension: "plist")!

let data = try! Data(contentsOf: url)

let categories = try! PropertyListDecoder().decode([Category].self, from: data)

The disadvantage of this approach is that you have to define extra types.

Approach #2: Load the default data into Core Data directly

I would use PropertyListSerialization to load the file (which works with both dictionaries and arrays):

let url = Bundle.main.url(forResource: "demoExercises", withExtension: "plist")!

let data = try! Data(contentsOf: url)

let categories = try! PropertyListSerialization.propertyList(from: data, format: nil)

    as! [[String: Any]]

Instead of the addToXXX methods I would use the inverse relationships to connect the objects, e.g.

category.addToExercises(exercise)

becomes

exercise.category = category

Together with the previously mentioned points the loop to read the property list contents into Core Data would look like this:

for category in categories {

    let cdcategory = Category(context: backgroundContext)

    cdcategory.title = category["title"] as! String

    cdcategory.imageName = category["imageName"] as! String



    for exercise in category["exercises"] as! [[String: Any]] {

        let cdexercise = Exercise(context: backgroundContext)

        cdexercise.category = cdcategory // Add exercise to the category

        cdexercise.question = exercise["question"] as! String

        // ...



        for answers in exercise["answers"] as! [[String: Any]] {

            let cdanswer = Answer(context: backgroundContext)

            cdanswer.exercise = cdexercise // Add answer to the exercise

            // ...

        }

    }

}

which is a bit shorter and easier to read than your original code.

One could also try to load the managed objects directly from the property list as in approach #1. That requires to add Decodable conformance to the NSManagedObject subclasses. A possible solution is described in How to use swift 4 Codable in Core Data? on Stack Overflow. However, it requires to write all the

 convenience init(from decoder: Decoder)

methods, so it may not be worth the effort.