I'm using PHP to send data in JSON to create an array of custom objects in Swift. I've been told retrieveData() is bad code because it's in an infinite loop. I'm not sure on how to clean it up so it can be more efficient. On a side note, is there a way I can save the custom objects in an array that is easier to save to UserDefaults without NSCoder?

The app is basically a blog reader. It gets data from a server's database and displays it in a tableview.

MainController.swift

var jsonArray: NSMutableArray = []
var mainArray = [Blog]()

// Retrieving Data from Server
func retrieveData() {

    let getDataURL = "http://example.com/receiving.php"
    let url: NSURL = NSURL(string: getDataURL)!

    do {
        let data: Data = try Data(contentsOf: url as URL)
        jsonArray = try JSONSerialization.jsonObject(with: data, options: .mutableContainers) as! NSMutableArray

        // Looping through jsonArray
        for i in 0..<jsonArray.count {

        // Create Blog Object
        guard let bID: String = (jsonArray[i] as AnyObject).object(forKey: "id") as? String,
            let bName: String = (jsonArray[i] as AnyObject).object(forKey: "blogName") as? String,
            let bStatus1: String = (jsonArray[i] as AnyObject).object(forKey: "blogStatus1") as? String,
            let bStatus2: String = (jsonArray[i] as AnyObject).object(forKey: "blogStatus2") as? String,
            let bURL: String = (jsonArray[i] as AnyObject).object(forKey: "blogURL") as? String,
            let bType: String = (jsonArray[i] as AnyObject).object(forKey: "blogType") as? String,
            let bDate: String = (jsonArray[i] as AnyObject).object(forKey: "blogDate") as? String,
            let bPop: String = (jsonArray[i] as AnyObject).object(forKey: "blogPop") as? String

            else {
                print("Error: (Creating Game Object)")
                return
            }

            // Add Blog Objects to mainArray
            mainArray.append(Blog(blogName: bName, andBlogStatus1: bStatus1, andBlogStatus2: bStatus2, andBlogURL: bURL, andBlogID: bID, andBlogType: bType, andBlogDate: bDate, andBlogPop: bPop))
        }
    }
    catch {
        print("Error: (Retrieving Data)")
    }
    myTableView.reloadData()
}

Blog.swift - Handles the Blog custom objects that it gets from retrieveData

import UIKit

class Blog: NSObject {

// Strings
var blogName: String
var blogStatus1: String
var blogStatus2: String
var blogURL: String
var blogID: String
var blogType: String
var blogDate: String
var blogPop: String

// Converting Strings into Objects
init(blogName bName: String,
     andBlogStatus1 bStatus1: String,
     andBlogStatus2 bStatus2: String,
     andBlogURL bURL: String,
     andBlogID bID: String,
     andBlogType bType: String,
     andBlogDate bDate: String,
     andBlogPop bPop: String)
{
    self.blogName = bName
    self.blogStatus1 = bStatus1
    self.blogStatus2 = bStatus2
    self.blogURL = bURL
    self.blogID = bID
    self.blogType = bType
    self.blogDate = bDate
    self.blogPop = bPop
    super.init()
  }
}
up vote 1 down vote accepted

Let me try to clean it little bit.

1 First of all you made jsonArray global. I think it's not necessary.
You can declare it locally where needed.

2 retrieveData() function looks very messy.
You can abstract that creation of Blog object.

3 You can improve for loop code

So here is what I would propose:

class Blog: NSObject {

  var blogName: String!
  var blogStatus1: String!
  var blogStatus2: String!
  var blogURL: String!
  var blogID: String!
  var blogType: String!
  var blogDate: String!
  var blogPop: String!

  init?(from jsonObject: AnyObject) {
    guard let bID: String = jsonObject.object(forKey: "id") as? String,
        let bName: String = jsonObject.object(forKey: "blogName") as? String,
        let bStatus1: String = jsonObject.object(forKey: "blogStatus1") as? String,
        let bStatus2: String = jsonObject.object(forKey: "blogStatus2") as? String,
        let bURL: String = jsonObject.object(forKey: "blogURL") as? String,
        let bType: String = jsonObject.object(forKey: "blogType") as? String,
        let bDate: String = jsonObject.object(forKey: "blogDate") as? String,
        let bPop: String = jsonObject.object(forKey: "blogPop") as? String else {
            print("Error: (Creating Game Object)")
            return nil
    }
    blogName = bName
    blogStatus1 = bStatus1
    blogStatus2 = bStatus2
    blogURL = bURL
    blogID = bID
    blogType = bType
    blogDate = bDate
    blogPop = bPop
    super.init()
  }
}

So outside world will not have to worry about converting json object into Blog.

Then simply use this abstraction as:

var mainArray = [Blog]()

func retrieveData() {

  let getDataURL = "http://example.com/receiving.php"
  let url: NSURL = NSURL(string: getDataURL)!

  do {
    let data: Data = try Data(contentsOf: url as URL)
    let jsonArray = try JSONSerialization.jsonObject(with: data, options: .mutableContainers) as! NSMutableArray

    // Looping through jsonArray
    for jsonObject in jsonArray {
      if let blog = Blog(from: jsonObject as AnyObject) {
        mainArray.append(blog)
      }
    }
  }
  catch {
    print("Error: (Retrieving Data)")
  }
  myTableView.reloadData()
}
  • I would make it a failable initializer: init?(from jsonObject: AnyObject) . – Martin R Jun 13 '17 at 19:40
  • Yes, would be more cleaner way; I forgot this approach, I'll mention it – D4ttatraya Jun 13 '17 at 20:17
  • A) I would retrieveData should only return an array of Blog. In an other method I would call this and set then thel mainArray with the result and reload data. If you change your source then you can easy exchange the call to retrieveData – muescha Jun 16 '17 at 1:40
  • B) mainData is not set to empty before filling it up – muescha Jun 16 '17 at 1:41

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.