Completed Assignment - Cora Jacobson #146
Conversation
swift-student
left a comment
swift-student
left a comment
There was a problem hiding this comment.
Great start on this project, Cora.
|
|
||
| import Foundation | ||
|
|
||
| class GigController { |
There was a problem hiding this comment.
This class is looking nice and clean so far
| switch loginType { | ||
| case .signUp: | ||
| gigController?.signUp(with: user, completion: { (result) in | ||
| do { | ||
| let success = try result.get() | ||
| if success { | ||
| DispatchQueue.main.async { | ||
| let alertController = UIAlertController(title: "Sign up successful!", message: "Please log in.", preferredStyle: .alert) | ||
| let alertAction = UIAlertAction(title: "OK", style: .default, handler: nil) | ||
| alertController.addAction(alertAction) | ||
| self.present(alertController, animated: true) { | ||
| self.loginType = .signIn | ||
| self.signInSegmentedControl.selectedSegmentIndex = 1 | ||
| self.signInButton.setTitle("Sign In", for: .normal) | ||
| } | ||
| } | ||
| } | ||
| } catch { | ||
| print("Error signing up: \(error)") | ||
| } | ||
| }) | ||
| case .signIn: | ||
| gigController?.signIn(with: user, completion: { (result) in | ||
| do { | ||
| let success = try result.get() | ||
| if success { | ||
| DispatchQueue.main.async { | ||
| self.dismiss(animated: true, completion: nil) | ||
|
|
||
| } | ||
| } | ||
| } catch { | ||
| if let error = error as? GigController.NetworkError { | ||
| switch error { | ||
| case .failedSignIn: | ||
| print("Sign in failed") | ||
| case .noData, .noToken: | ||
| print("No data received") | ||
| default: | ||
| print("Other error occurred") | ||
| } | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
This is a rather large switch statement, maybe a good place to refactor out into a couple smaller functions.
swift-student
left a comment
swift-student
left a comment
There was a problem hiding this comment.
Great job on this project, Cora. The UI looks nice with the custom color, and you did the stretch goal. Your network calls all look good. Like I noted, don't be afraid to add some whitespace to separate logical chunks of code, and also consider refactoring tasks out to smaller functions to make things more easily readable.
| do { | ||
| let jsonData = try JSONEncoder().encode(user) | ||
| print(String(data: jsonData, encoding: .utf8)!) | ||
| request.httpBody = jsonData | ||
| let task = URLSession.shared.dataTask(with: request) { (_, response, error) in | ||
| if let error = error { | ||
| print("SignUp failed with error: \(error)") | ||
| completion(.failure(.failedSignUp)) | ||
| return | ||
| } | ||
| guard let response = response as? HTTPURLResponse, | ||
| response.statusCode == 200 else { | ||
| print("Sign up was unsuccesful") | ||
| completion(.failure(.failedSignUp)) | ||
| return | ||
| } | ||
| completion(.success(true)) | ||
| } | ||
| task.resume() | ||
| } catch { | ||
| print("Error encoding user: \(error)") | ||
| completion(.failure(.failedSignUp)) | ||
| } |
There was a problem hiding this comment.
I would prefer to only wrap the do catch block around the try statement so it is a little more clear what is going to throw an error. Nothing inherently wrong with this code, just a matter of style.
| if let title = titleTextField.text, | ||
| !title.isEmpty, | ||
| let description = descriptionTextView.text, | ||
| !description.isEmpty { | ||
| let gig = Gig(title: title, dueDate: datePicker.date, description: description) | ||
| if let gigController = gigController { | ||
| if gigController.gigs.contains(gig) { | ||
| let alert = UIAlertController(title: "This gig is already saved.", message: nil, preferredStyle: .alert) | ||
| let okButton = UIAlertAction(title: "OK", style: .cancel, handler: nil) | ||
| alert.addAction(okButton) | ||
| present(alert, animated: true, completion: nil) | ||
| } else { | ||
| gigController.createGig(with: gig, completion: { (result) in | ||
| switch result { | ||
| case .success(true): | ||
| DispatchQueue.main.async { | ||
| self.navigationController?.popViewController(animated: true) | ||
| } | ||
| case .failure(let error): | ||
| print("Error creating gig: \(error)") | ||
| default: | ||
| return | ||
| } | ||
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't be afraid to add some whitespace to group things into logical chunks. As with the switch statement I pointed out, this chunk of code might be a good place to refactor into a couple smaller functions for clarity.
| if gigController.gigs.contains(gig) { | ||
| let alert = UIAlertController(title: "This gig is already saved.", message: nil, preferredStyle: .alert) | ||
| let okButton = UIAlertAction(title: "OK", style: .cancel, handler: nil) | ||
| alert.addAction(okButton) | ||
| present(alert, animated: true, completion: nil) | ||
| } else { |
There was a problem hiding this comment.
Nice job doing the stretch goal!
2 participants
@swift-student