Skip to content

0=0#5

Open
2underground wants to merge 2 commits intomainfrom
sprint_12
Open

0=0#5
2underground wants to merge 2 commits intomainfrom
sprint_12

Conversation

@2underground
Copy link
Owner

No description provided.

Comment on lines +14 to +24
init(_ photoResult: PhotoResult, date: ISO8601DateFormatter) {
self.id = photoResult.id
self.size = CGSize(width: photoResult.width, height: photoResult.height)
self.createdAt = date.date(from: photoResult.createdAt ?? "")
self.welcomeDescription = photoResult.description
self.thumbImageURL = photoResult.urls.thumb
self.largeImageURL = photoResult.urls.full
self.regularImageURL = photoResult.urls.regular
self.smallImageURL = photoResult.urls.small
self.isLiked = photoResult.likedByUser
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Наверное, лучше будет вомпользоваться обычным init-ом, где принимаются все параметры структуры. Т. е.:

    init(
        id: String,
        size: CGSize,
        ...
    ) {
        self.id = id
        self.size = size
        ...
    }

Это поможет уменьшить связанность структур, что позволит не создовать PhotoResult, когда это не нужно (как происходить при изменении лайка), => упростит тестирование кода использующего структуру и использование данной структуры


private let urlSession = URLSession.shared

private let dateFormatter = ISO8601DateFormatter()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Классно, что воспользовался именно ISO8601DateFormatter)


static let shared = ImagesListService()

static let didChangeNotification = Notification.Name(rawValue: "ImagesListServiceDidChange")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥, что с маленькой буквы называется константа. Так, стиль её названия совпадает с остальным кодом)


guard currentTask == nil else { return }

let nextPage = lastLoadedPage == nil ? 1 : lastLoadedPage! + 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно переформулировать, чтобы лишний раз не использовать force unwrap:
let nextPage = (lastLoadedPage ?? 0) + 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants