-
Notifications
You must be signed in to change notification settings - Fork 17
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
Resolve Plutakhin tests #42
base: master
Are you sure you want to change the base?
Conversation
.gitignore
Outdated
@@ -7,3 +7,4 @@ | |||
/spec/reports/ | |||
/tmp/ | |||
/log/ | |||
/.idea |
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.
нужно настроить редактор так, чтобы в нем по сохранению в конце файла оставалась одна пустая строка
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.
так же .idea - это файл, который создается твоей средой разработки. такие настройки выносятся в твой глобальный .gitignore, а не в .gitignore проекта
test/plutakhin/arrays/solution.rb
Outdated
class << self | ||
def replace(array) | ||
max = array.max | ||
array.map! { |a| a >= 0 ? max : a } |
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.
map! мутирует исходный массив, тебе тут это совершенно не нужно, можно использовать map без !
test/plutakhin/arrays/solution.rb
Outdated
value = array[mid] | ||
return mid if value == query | ||
|
||
if query < value |
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.
тут можно использовать тернарный оператор
test/plutakhin/fp/solution.rb
Outdated
def rating(films) | ||
ratios = films.map do |film| | ||
film['rating_kinopoisk'].to_f if !film['rating_kinopoisk'].to_f.zero? && !film['country'].nil? && film['country'].split(',').count > 1 | ||
end . compact |
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.
какие-то странные пробелы с точками
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.
если какой-то блок заканчивается end, после него уже цепочку методов не строим.
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.
условие в if очень большое, строка плохо читается, можно вынести его в переменную или вспомогательный метод
test/plutakhin/fp/solution.rb
Outdated
end | ||
|
||
def chars_count(films, threshold) | ||
total = 0 |
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.
вместо глобального счетчика можно использовать reduce
test/plutakhin/fp/test.rb
Outdated
# Посчитать средний рейтинг фильмов по версии кинопоиска у которых две или больше стран | ||
# Фильмы у которых рейтиг не задан или равен 0 не учитывать в расчете среднего. | ||
def test_rating | ||
skip |
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.
тесты заскипаны, они не запускаются
test/plutakhin/fp2/solution.rb
Outdated
# Написать свою функцию my_compact | ||
def my_compact | ||
array = MyArray.new | ||
for elm in self |
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.
тут можно использовать my_each вместо for. в обычной жизни вместо for всегда используется each
ignore_first = true | ||
acc = first | ||
end | ||
index = 0 |
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.
можно порефакторить и не использовать никаких счетчиков. также предлагаю пользоваться своими функциями типа my_each
test/plutakhin/fp2/test.rb
Outdated
end | ||
|
||
def test_my_each | ||
skip |
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.
тесты скипнуты, читай ридми
Обновил реализацию. |
array = CSV.readlines('./test/fixtures/films.csv', headers: true) | ||
|
||
result = Plutakhin::Fp.chars_count(array, 5) | ||
assert result == 891 |
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.
да, значения в тесте неправильные, нужны 3966 и 42
No description provided.