Skip to content

request#1

Open
DmitriiOS wants to merge 31 commits into
initialfrom
main
Open

request#1
DmitriiOS wants to merge 31 commits into
initialfrom
main

Conversation

@DmitriiOS
Copy link
Copy Markdown
Owner

Code review, please!

@DmitriiOS DmitriiOS requested a review from svallex November 12, 2020 06:41
@DmitriiOS DmitriiOS self-assigned this Nov 12, 2020
Comment thread testApp/Model/Entities/DatesAndStars.swift Outdated
Comment thread testApp/Model/Entities/GitUserData.swift
Comment thread testApp/Model/Entities/RealmModels.swift
Comment thread testApp/Model/Entities/GitUserData.swift Outdated

struct GithubStarDates {
let starDatesID: String
let dates: String
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

dates: String смущает. Даты во множественном числе в виде строки... Что сохраняется в этой переменной?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

dates исправил на date

Comment thread testApp/Model/Entities/UserSettings.swift Outdated
Comment thread testApp/Scenes/Home/HomePresenter.swift
Comment thread testApp/Scenes/Home/HomePresenter.swift
Comment thread testApp/Scenes/Home/HomeViewController.swift Outdated
Comment thread testApp/Scenes/Home/HomeViewController.swift Outdated
Comment thread testApp/Scenes/Second/SecondPresenter.swift Outdated
Comment thread testApp/Scenes/Second/SecondPresenter.swift Outdated
Comment thread testApp/Workers/StarDatesService.swift Outdated
Comment thread testApp/Workers/StarDatesService.swift Outdated
reloadDispatchGroup.leave()
}
}

Copy link
Copy Markdown
Collaborator

@svallex svallex Nov 12, 2020

Choose a reason for hiding this comment

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

Хороший потенциал для рефакторинга: метод получился довольно большой, выполняет несколько задач, большая вложенность блоков...
Просматривается разделение на отдельные методы (или даже сущности): подготовка запроса, парсинг моделей и т.д.

func getCurrentRepositoryInfo(currentRepositoryInfo: CurrentRepositoryInfo)
func reloadRepoStars(_ starDates: [RepoStarsByDates])
func whenAllDataIsReady()
func activityIndicatorStop()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Данные для второго экрана лучше получать из второго презентера, а не передавать из первого

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.

3 participants