-
Notifications
You must be signed in to change notification settings - Fork 0
sprint13 is done #6
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?
Conversation
imagesListService.changeLike(photoId: photo.id, isLike: !photo.isLiked) { [weak self] result in | ||
guard let self else { return } | ||
switch result { | ||
case .success: | ||
self.photos = self.imagesListService.photos | ||
cell.setIsLiked(isLiked: self.photos[indexPath.row].isLiked) | ||
|
||
view?.dismissBlockingHud() | ||
case .failure: | ||
print("Function: \(#function), line \(#line) Failed to change Like") | ||
view?.showBlockingHud() | ||
} | ||
} |
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.
Неточности в реализации MVP. Presenter знает о ImagesListCell
, умеет настраивать ячейку. Это обязанность View. Presenter не должен знать ничего о конкретной реализации UI.
private func updateAvatar(placeholder: Placeholder) { | ||
guard let profileImageURL = ProfileImageService.shared.avatarURL, | ||
let url = URL(string: profileImageURL) else { return } | ||
let processor = RoundCornerImageProcessor(cornerRadius: 35) | ||
view?.profileImage.kf.setImage(with: url, | ||
placeholder: placeholder, | ||
options: [.processor(processor)]) | ||
} |
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.
Использование библиотек работы с UI(здесь Kingfisher) тоже не соответствует принципам MVP. Лучше перенести настройку аватара во View, т.е. контроллер.
let processor = RoundCornerImageProcessor(cornerRadius: 35) | ||
view?.profileImage.kf.setImage(with: url, | ||
placeholder: placeholder, |
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.
RoundCornerImageProcessor из Kingfisher скругляет именно ту картинку, которую скачал. Т.е., если он скачал большую картинку(например, 2000х2000), то он скруглит у нее только 20 поинтов(такое число указано в коде проекта), что вряд ли будет заметно. Всегда учитывай это при использовании RoundCornerImageProcessor.
import UIKit | ||
import Kingfisher | ||
|
||
final class ProfileViewController: UIViewController { | ||
protocol ProfileViewControllerProtocol: AnyObject { | ||
var presenter: ProfileViewPresenterProtocol? { get set } | ||
var profileImage: UIImageView { get } | ||
var nameLabel: UILabel { get } | ||
var bioLabel: UILabel { get } | ||
var loginLabel: UILabel { get } | ||
} |
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.
Здесь тоже неточности в реализации MVP. По-хорошему Presenter должен передать во View модель профиля. А View отобразить так, как считает нужным. Presenter'у не нужно ничего знать о UIImageView
, UILabel
и прочих компонентах.
Плюс использование библиотек работы с UI(здесь Kingfisher) тоже не соответствует принциам MVP.
No description provided.