Swift project using PHP web service

I was hoping for someone to review my current project, which was created in Swift and uses a PHP web service. I'm not worried about UI elements, as this is just a 'test' project, but I'm concerned about two things: using the best practices and security. I'm concerned that the SQL query is not safe, among other things.

user.swift

```
import Foundation

class User: NSObject {

var firstName: String?
var lastName: String?
var username: String
var password: String
var email: String?

var recievedJSON: NSMutableData = NSMutableData()
var userData: [[String: String]]!

var verified: Bool = false

required init(username: String, password: String) {
self.username = username
self.password = password
}

init(firstName: String, lastName: String, username: String, password: String, email: String) {
self.firstName = firstName
self.lastName = lastName
self.username = username
self.password = password
self.email = email
}

func attemptRegister() {
var variables: [String] = ["firstname=" + self.firstName! + "&"]
variables.append("lastname=" + self.lastName! + "&")
variables.append("username=" + self.username + "&")
variables.append("password=" + self.password + "&")
variables.append("email=" + self.email!)
request("https://codekaufman.com/register.php", variables: variables)
}

func attemptSignIn() {
var variables: [String] = ["username=" + self.username + "&"]
variables.append("password=" + self.password)
request("https://codekaufman.com/login.php", variables: variables)
println("Attempting sign-in...")
}

private func request(urlPath: String, variables: [String]?) {
var url: NSURL = NSURL(string: urlPath)!
var request: NSMutableURLRequest = NSMutableURLRequest(URL: url)

if(variables != nil) {
request.HTTPMethod = "POST"

var body

I'm concerned that the SQL query is not safe, among other things.

Well, you're right to be concerned. I don't know much about PHP (basically nothing), so I'll let someone who does know about PHP comment about that half of your code.

But from the Swift end, we're passing the password as plaintext. We really shouldn't be doing this. When we send passwords over any sort of networking request, the password should be encrypted in some way. MD5 is a common approach. I'm not a safety expert and couldn't say one way or the other whether or not MD5 is still appropriate or not, but anything is better than passing completely unencrypted passwords.

func attemptRegister() {
    var variables: [String] = ["firstname=" + self.firstName! + "&"]
    variables.append("lastname=" + self.lastName! + "&")
    variables.append("username=" + self.username + "&")
    variables.append("password=" + self.password + "&")
    variables.append("email=" + self.email!)
    request("https://codekaufman.com/register.php", variables: variables)
}

This method will crash every time it's call and one of your optional properties is nil.

If these properties truly are optional, then we need to optionally unwrap them:

if let email = self.email {
    variables.append("email=" + email)
}

And moreover, appending the & to the end of each of these variables here seems really contrived.

Moreover, why do we need to build the array here and pass it into another method... when the array is built of nothing more than instance variables that the other method has access to?

To be honest, if this is me, I think I completely rethink and restructure this class. You call this class User, but it's nothing more than a utility for signing in or registering with your service. How usable is this object after the user has either signed in or registered? And should we REALLY be storing a password for as long as this object is kept around?

Realistically, our username/password (and in the case of registering, the other information) should be passed to a function, even if it's a class function of the user class. As soon as we send the information to the web service, we should forget about it within the app.

And then, assuming a successful response from the web service, our User class should be instantiated with the information about this user that our app actually needs to use. A userid? Display name, profile picture, etc? These are the things our User class should keep as instance variables--the information that our application needs AFTER the user has successfully signed in or registered. And we definitely should not be hanging on to information we need for signing in or registering (except any of this information that is also information that our app needs to function).

This bit of code by the way is a little redundant:

if (userData != nil) {
    println("NSData had data, printing and returning.")
    println(userData)
    return userData
} else {
    println("NSData empty, returning nil.")
    return nil
}

The only reason to actually split this into an if/else is because you want to print this stuff out for diagnostic reasons. But realistically, in the final version of your app, you won't want to print this stuff out... and as such, the if/else could be eliminated.

As such, I'd rewrite this section as:

#if DEBUG
    if (userData != nil) {
        println("NSData had data, printing and returning.")
        println(userData)
    } else {
        println("NSData empty, returning nil.")
    }
#endif

return userData

We don't need to return a literal nil when userData is nil. If userData is nil, then the statement return userData will already return nil for us!

StackExchange Code Review Q#74732, answer score: 3