-
Couldn't load subscription status.
- Fork 0
Sprint 12 #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Sprint 12 #4
Conversation
|
|
||
| import UIKit | ||
|
|
||
| class AuthViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
Не забывайте проставлять данное ключевое слово для ускорения работы компилятора. Это оптимизация его работы. В таком случае он будет быстрее при вызове функции находить адрес ячейки памяти, где лежит реализации вызванной функции.
https://habr.com/ru/post/673400/
| case tokenKey = "BearerToken" | ||
| } | ||
|
|
||
| class OAuth2TokenStorage { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
| } | ||
|
|
||
| final class OAuth2Service { | ||
| static let shared = OAuth2Service() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Раз синглтон, то должен быть
private init() {}
ImageFeed/Auth/OAuth2Service.swift
Outdated
|
|
||
| guard let window = UIApplication.shared.windows.first else { fatalError("Invalid Configuration") } | ||
| let splashViewController = SplashViewController() | ||
| window.rootViewController = splashViewController |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ой-ей-ей. Сервис не должен никак касаться презентейшена и порождать контроллеры. Тем более устанавливать их)
Я бы советовал тебе здесь комплишн пробросить наверх, и тот, кто вызвал этот метод, как раз и устанавливал новый рутовый контроллер в своей кложуре
| case .success(let body): | ||
| let authToken = body.accessToken | ||
| self.authToken = authToken | ||
| completion(.success(authToken)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вот здесь же ты комплишн передаешь наверх)
| // | ||
|
|
||
| import Foundation | ||
| import UIKit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UIKit содержит в себе Foundation. Поэтому оставляй только UIKit
| final class WebViewViewController: UIViewController { | ||
|
|
||
| @IBOutlet private weak var webView: WKWebView! | ||
| @IBOutlet weak var progressView: UIProgressView! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему одна приватная, а другая internal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не должен прогресс наружу торчать
| let SecretKey = "b99lHBvBrUgVpF70lQFrvKJ3hyyCCQMA0tk28nVoTB4" | ||
| let RedirectURI = "urn:ietf:wg:oauth:2.0:oob" | ||
| let AccessScope = "public+read_user+write_likes" | ||
| let DefaultBaseURL = URL(string: "https://api.unsplash.com")! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
С маленькой буквы, это же не типы данных)
|
|
||
| import Foundation | ||
|
|
||
| class ImageListService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
| request = makeRequest(path: "/photos/\(photoId)/like", httpMethod: "POST", baseURL: DefaultBaseURL) | ||
| } else { | ||
| request = makeRequest(path: "/photos/\(photoId)/like", httpMethod: "DELETE", baseURL: DefaultBaseURL) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тернарный оператор лучше будет тут + isLike и так булево значение, не нужно его сравнивать с true/false
true == true -> true
false == true -> false
То есть можно просто записать
if isLike { ... }
| import UIKit | ||
| import Kingfisher | ||
|
|
||
| class ImagesListCell: UITableViewCell { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
| import UIKit | ||
| import Kingfisher | ||
|
|
||
| class ImagesListViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
|
|
||
| class ImagesListViewController: UIViewController { | ||
|
|
||
| @IBOutlet private var tableViewImage: UITableView! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private weak var
|
|
||
| final class ProfileImageService { | ||
|
|
||
| static let shared = ProfileImageService() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private init() {}
| import UIKit | ||
| import Kingfisher | ||
|
|
||
| class ProfileViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
|
|
||
| import UIKit | ||
|
|
||
| class SingleImageViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final
| @IBOutlet weak var imageView: UIImageView! | ||
| @IBOutlet weak private var scrollView: UIScrollView! | ||
|
|
||
| @IBOutlet weak var sharingButton: UIButton! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все приватные должны быть
| UIBlockingProgressHUD.dismiss() | ||
| self.switchToTabBarController() | ||
| case .failure: | ||
| UIBlockingProgressHUD.dismiss() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Дублируется код. Можно вынести после switch
ImageFeed/Auth/OAuth2Service.swift
Outdated
| URLQueryItem(name: "code", value: code), | ||
| URLQueryItem(name: "grant_type", value: "authorization_code") | ||
| ] | ||
| let url = urlComponents.url! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В продакшн коде не используется принудительное извлечение значения. Разворачивай опционалы безопасно, пожалуйста
| case width = "width" | ||
| case height = "height" | ||
| case color = "color" | ||
| case likes = "likes" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не нужно указывать значения кейсам, названия которых совпадают) Только тем, поля которых отличаются в json
No description provided.