4
\$\begingroup\$

I implemented an extension for the NSManagedObjectContext that takes any CoreData class as a parameter and performs a fetch request on it.

I am new to CoreData and would very much appreciate any sort of feedback for this function

extension NSManagedObjectContext{
func fetchObjects <T: NSManagedObject>(_ entityClass:T.Type, sortBy: [NSSortDescriptor]? = nil, predicate: NSPredicate? = nil) throws -> [AnyObject]{
    var request: NSFetchRequest<NSFetchRequestResult>

    if #available(iOS 10.0, *) {
        request = entityClass.fetchRequest()
    } else {
        let entityClassName = NSStringFromClass(entityClass)
        let entityName = entityClassName.components(separatedBy: ".").last ?? entityClassName
        request = NSFetchRequest(entityName: entityName)
    }

    var fetchRequestError: Error?
    request.returnsObjectsAsFaults = false

    if let predicate = predicate {
        request.predicate = predicate
    }
    if let sortBy = sortBy {
        request.sortDescriptors = sortBy
    }

    var fetchedResult: [T]?

        do {
            fetchedResult = try self.fetch(request) as? [T]
        }
        catch let error {
            fetchRequestError = error
            //print(fetchRequestError)
        }

    guard let entityArray = fetchedResult else {
        throw fetchRequestError!
    }

    return entityArray
    }
}
\$\endgroup\$
5
\$\begingroup\$

Your code looks good, but there are some things which can be simplified. Additionally, the method can be improved to return a array of the actual managed object subclass type instead of AnyObject.

Simplifications and small improvements

var request: NSFetchRequest<NSFetchRequestResult>

request is assigned exactly once, therefore it should be a constant:


let request: NSFetchRequest<NSFetchRequestResult>
let entityClassName = NSStringFromClass(entityClass)
let entityName = entityClassName.components(separatedBy: ".").last ?? entityClassName

can be simplified to

let entityName = String(describing: entityClass)

which returns the unqualified class name of the given class (in contrast to String(reflecting:) which returns the fully qualified class name including the module name).


if let predicate = predicate {
    request.predicate = predicate
}
if let sortBy = sortBy {
    request.sortDescriptors = sortBy
}

The predicate and sortDescriptors of NSFetchRequest are optionals, so there is no need to conditionally unwrap the given parameters. You can assign them directly:

request.predicate = predicate
request.sortDescriptors = sortBy

fetchedResult = try self.fetch(request) as? [T]

We know that the fetch request returns an array of T objects, the cast cannot fail. Therefore the second half of the method can be reduced to

do {
    let fetchedResult = try self.fetch(request) as! [T]
    return fetchedResult
} catch let error {
    print(error.localizedDescription)
    throw error
}

making the fetchRequestError variable obsolete. And if you don't need to print the error locally then no do/catch is needed at all:

let fetchedResult = try self.fetch(request) as! [T]
return fetchedResult

A possible error will be re-thrown to the caller of the method.


Putting it all together, the method now looks like this:

extension NSManagedObjectContext{
    func fetchObjects <T: NSManagedObject>(_ entityClass:T.Type,
                       sortBy: [NSSortDescriptor]? = nil,
                       predicate: NSPredicate? = nil) throws -> [AnyObject] {

        let request: NSFetchRequest<NSFetchRequestResult>
        if #available(iOS 10.0, *) {
            request = entityClass.fetchRequest()
        } else {
            let entityName = String(describing: entityClass)
            request = NSFetchRequest(entityName: entityName)
        }

        request.returnsObjectsAsFaults = false
        request.predicate = predicate
        request.sortDescriptors = sortBy

        let fetchedResult = try self.fetch(request) as! [T]
        return fetchedResult
    }
}

Returning a "correctly" typed array

The return type is [AnyObject], which means that the return value has to be cast to the actual managed object subclass type

let objects = try context.fetchObjects(YourEntity.self) as! [YourEntity]

This can be improved by changing the return type to [T] and using a typed fetched request NSFetchRequest<T>:

extension NSManagedObjectContext{
    func fetchObjects <T: NSManagedObject>(_ entityClass:T.Type,
                       sortBy: [NSSortDescriptor]? = nil,
                       predicate: NSPredicate? = nil) throws -> [T] {

        let request: NSFetchRequest<T>
        if #available(iOS 10.0, *) {
            request = entityClass.fetchRequest() as! NSFetchRequest<T>
        } else {
            let entityName = String(describing: entityClass)
            request = NSFetchRequest(entityName: entityName)
        }

        request.returnsObjectsAsFaults = false
        request.predicate = predicate
        request.sortDescriptors = sortBy

        let fetchedResult = try self.fetch(request)
        return fetchedResult
    }
}

This is now called as

let objects = try context.fetchObjects(YourEntity.self)

and objects is an array of YourEntity without additional cast.

\$\endgroup\$
0
\$\begingroup\$

It looks solid to me, but I can't comment on performance-wise. My issue was trying to read it. It took me a few minutes of figuring it out before I had to reformat it in Xcode:

extension NSManagedObjectContext {

    func fetchObjects <T: NSManagedObject>(_ entityClass: T.Type,
                                             sortBy: [NSSortDescriptor]? = nil,
                                             predicate: NSPredicate? = nil)
     throws -> [AnyObject] {

        var request: NSFetchRequest<NSFetchRequestResult>

        if #available(iOS 10.0, *) {
            request = entityClass.fetchRequest()
        } else {
            let entityClassName = NSStringFromClass(entityClass)
            let entityName = entityClassName.components(separatedBy: ".").last
                             ?? entityClassName

            request = NSFetchRequest(entityName: entityName)
        }

        var fetchRequestError: Error?
        request.returnsObjectsAsFaults = false

        if let predicate = predicate {
            request.predicate = predicate
        }

        if let sortBy = sortBy {
            request.sortDescriptors = sortBy
        }

        var fetchedResult: [T]?

        do {
            fetchedResult = try self.fetch(request) as? [T]
        }
        catch let error {
            fetchRequestError = error
            //print(fetchRequestError)
        }

        guard let entityArray = fetchedResult else {
            throw fetchRequestError!
        }

        return entityArray
    }
}

I know your style is definitely different from mine, ( I would wrap each thing up in a labeled do block, or comment each section based on its purpose) but I think that mine is considerably easier to read, and I tried to respect your coding style and the Swift conventions.

A couple places you had inconsistent spacing and linebreaks

My two biggest issues were the single line func declaration, and your nil coalesce.. I didn't realize until after I'd reformatted that you had actually assigned a value to request after that (it all looked like one line to me)

Just my 2c.. I know I tend to over-linebreak in my code, but for me, it took me a while to figure out what was going on in your code initially. A few extra breaks can help emphasize when you are starting a new logic sequence, or when you are setting one one up vs actually defining it.

Overall an "A" I think! :D Not that mine is any better. Just my take on it.

Hope this helps.


PS, here is my personal rendition of it:

extension NSManagedObjectContext {

    func fetchObjects<T: NSManagedObject> (_ entityClass: T.Type,
                                             sortBy:      [NSSortDescriptor]? = nil,
                                             predicate:   NSPredicate?        = nil)
     throws -> [AnyObject] {

        // Initial request:
        var request: NSFetchRequest<NSFetchRequestResult>

        if #available(iOS 10.0, *) {
            request = entityClass.fetchRequest()
        }

        else {
            let entityClassName = NSStringFromClass(entityClass)
            let entityName = entityClassName.components(separatedBy: ".").last
                                             ?? entityClassName

            request = NSFetchRequest(entityName: entityName)
        }

        // Fetch request:
        var fetchRequestError: Error?
        request.returnsObjectsAsFaults = false

        if let predicate = predicate { request.predicate = predicate }
        if let sortBy = sortBy { request.sortDescriptors = sortBy }

        var fetchedResult: [T]?

        do { fetchedResult = try self.fetch(request) as? [T] }
        catch let error { fetchRequestError = error }

        // Returner:
        guard let entityArray = fetchedResult
            else { throw fetchRequestError! }

        return entityArray
    }
}
\$\endgroup\$

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.