Conversation
| module Arrays | ||
| class << self | ||
| def replace(array) | ||
| max = 0 |
There was a problem hiding this comment.
если массив будет состоять из отрицательных элементов, в переменной max будет храниться некорректное значение
test/exercise/arrays/solution.rb
Outdated
| max = 0 | ||
| item = 0 | ||
| array.size.times { |n| max = array[n] if max < array[n] } | ||
| while array.size > item |
There was a problem hiding this comment.
в руби есть готовые методы, делающие примерно то же. Посмотри про методы итерации each, map и тп
test/exercise/arrays/solution.rb
Outdated
| def replace(array) | ||
| max = 0 | ||
| item = 0 | ||
| array.size.times { |n| max = array[n] if max < array[n] } |
There was a problem hiding this comment.
вынеси поиск максимума в отдельный метод
test/exercise/arrays/solution.rb
Outdated
| mid_index | ||
| when 1 | ||
| subs = search(array.drop(mid_index + 1), query) | ||
| subs.nil? ? nil : (mid_index + 1) + subs |
There was a problem hiding this comment.
что такое subs? возможно не оч подходящее название
|
Да я думал нельзя использовать map each. Ну теперь ещё проще
пт, 3 нояб. 2023 г., 13:28 George Yakimov ***@***.***>:
… ***@***.**** commented on this pull request.
------------------------------
In test/exercise/arrays/solution.rb
<#1 (comment)>
:
> @@ -2,11 +2,34 @@ module Exercise
module Arrays
class << self
def replace(array)
+ max = 0
+ item = 0
+ array.size.times { |n| max = array[n] if max < array[n] }
вынеси поиск максимума в отдельный метод
—
Reply to this email directly, view it on GitHub
<#1 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN3KB4MJJDUDMWIRK37RNLLYCSMJFAVCNFSM6AAAAAA63CSUTWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMJRHE2DENBYHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
@YJorge Привет! Проверь пожалуйста я по фиксил! |
test/exercise/arrays/solution.rb
Outdated
| def replace(array) | ||
| array | ||
| array.map do |item| | ||
| if item.positive? |
There was a problem hiding this comment.
вложенность ухудшает читаемость. Перепиши через guard clause/тернарный
test/exercise/arrays/solution.rb
Outdated
|
|
||
| mid_index = array.length / 2 | ||
|
|
||
| case query <=> array[mid_index] |
There was a problem hiding this comment.
- Не используй <=> оператор. Он усложняет восприятие и чтение кода.
- вложенность ухудшает читаемость. Перепиши через guard clause/тернарный
|
@akmatkulov 1. Если делаешь исправления и считаешь, что можно проводить ревью заново. Отправляй запрос через https://learn.dualboot.ru/ . Оповещение через комментарий может затеряться и ревью не произойдет до тех пор, пока на сайте не будет запроса |
|
@YJorge if |
в данном случае вложенность - наличие кода вида такие конструкции лучше дробить до вида одиночных условий(guard clause) или записывать в виде тернарных. За счет этого должна улучшиться читабельность кода. Да, не всегда получается сходу полностью избавиться от if/else, тогда нужно поискать то, что можно упростить |
| return search(array.take(mid_index), query) if query < array[mid_index] | ||
|
|
||
| min_index = search(array.drop(mid_index + 1), query) | ||
| min_index.nil? ? nil : (mid_index + 1) + min_index |
There was a problem hiding this comment.
- почему возвращается
nil? если элемент не найден, должно вернуться-1 - min_index = search(array.drop(mid_index + 1), query) почему это минимальный индекс? Кажется название не оч соответствует
- функция не работает. Попробуй проверить
search([1,2,4], 3)илиsearch([1,2,4,5,7], 6)
2 participants
No description provided.