Решение на Първа задача от Герасим Станчев

Обратно към всички решения

Към профила на Герасим Станчев

Резултати

  • 6 точки от тестове
  • 0 бонус точки
  • 6 точки общо
  • 12 успешни тест(а)
  • 0 неуспешни тест(а)

Код

def series_calculation(first_term, second_term, index)
return [first_term, second_term].max if index == 1
3.upto(index) do |series_step|
first_term, second_term =
sum_terms_over_series_step_parity(first_term, second_term, series_step)
end
index.even? ? first_term : second_term
end
def sum_terms_over_series_step_parity(first_term, second_term, series_step)
case series_step.even?
when true then first_term += second_term
when false then second_term += first_term
end
return first_term, second_term
end
def series(sequence_name, index)
case sequence_name
when 'fibonacci' then series_calculation(1, 1, index)
when 'lucas' then series_calculation(1, 2, index)
when 'summed'
series_calculation(1, 1, index) + series_calculation(1, 2, index)
end
end

Лог от изпълнението

............

Finished in 0.0133 seconds
12 examples, 0 failures

История (4 версии и 10 коментара)

Герасим обнови решението на 10.10.2014 15:08 (преди около 10 години)

+def calculate(first, second, index)
+ return [first, second].max if index == 1
+ result = 0
+ 2.upto(index) do
+ first, second, result = operations(first, second, result)
+ end
+ result
+end
+
+def operations(first, second, result)
+ result += first
+ first = second
+ second = result
+ return first, second, result
+end
+
+def series(string, index)
+ case string
+ when 'fibonacci' then calculate(1, 1, index)
+ when 'lucas' then calculate(1, 2, index)
+ when 'summed' then calculate(1, 1, index) + calculate(1, 2, index)
+ end
+end
  • Имаш малко проблеми с whitespaces, консултирай се със style guide-a, за да ги адресираш.
  • Трябва да поработиш над измислянето на имена. string, result, calculate, operations и т.н. са супер generic. Опитвай се да именуваш близо до домейна на проблема, който решаваш. Тук си представи, че някой математик трябва да се научи как да намира fibonacci, lucas и summed. Това, което ще види е:

"С помощта на низ и индекс разгледай дали низът е някое от трите и ако е, калкулирай съответно с 1, 1 и индекса, 1, 2 и индекса или направи предните две калкулации и ги събери. При калкулирането отговорът е максималното от първото и второто, ако индексът е 1. Иначе с резултат, който е 0, за всяко число от 2 до индекса, първото, второто и резултатът се получават като извършиш операцията върху тях. Операцията добавя към резултата първото, присвоява на първото второто и на второто резултата. Като приложиш операцията за всички въпросни числа, отговорът ти е резултатът."

Герасим Станчев

Ти разбра ли нещо - аз не. :) Стреми се кодът да се чете, не да се интерпретира наум.

  • Супер е, че си видял, че има множествено присвояване на един ред, но вероятно можеш да измислиш по-проста имплементация, която няма нужда от него. Всеки път, когато се извиква operations, скрито се създава нов масив. Не гоним оптималност на кода, но това е излишно spawn-ване на много short lived обекти в цикъл, не е добра практика.
  • Решението ти е доста процедурно, в Ruby по принцип, а и за конкретния проблем, всичко може да се опише много кратко и видно с по-функционален подход.

Благодаря ти за коментара! Помогна ми в ориентацията с езика. Мисля, че first и second трябва да останат в имената на променливите, защото те си остават first и second за определен елемент от реда, просто елемента нараства, а с него и те. Като цяло, промених подхода си и мисля, че сега кода е доста по-разбираем. Пускам нова версия и чакам коментари. : -)

  • П.П.: Странно е, че skeptic не хвана липсата на индентация при when-овете. Освен това, rubocop отчита индентацията като грешка. Moже би някои програмисти приемат конвенцията за липса на индентация, тъй като на case може и да не се подава аргумент.

Герасим обнови решението на 10.10.2014 21:51 (преди около 10 години)

-def calculate(first, second, index)
- return [first, second].max if index == 1
- result = 0
- 2.upto(index) do
- first, second, result = operations(first, second, result)
+def iterative_calculation(first_term, second_term, index)
+ return [first_term, second_term].max if index == 1
+ 2.upto(index) do |iterator|
+ iterator.even? ? first_term += second_term : second_term += first_term
end
- result
+ index.even? ? first_term : second_term
end
-def operations(first, second, result)
- result += first
- first = second
- second = result
- return first, second, result
-end
-
-def series(string, index)
- case string
- when 'fibonacci' then calculate(1, 1, index)
- when 'lucas' then calculate(1, 2, index)
- when 'summed' then calculate(1, 1, index) + calculate(1, 2, index)
+def series(sequence_name, index)
+ case sequence_name
+ when 'fibonacci' then iterative_calculation(0, 1, index)
+ when 'lucas' then iterative_calculation(1, 2, index)
+ when 'summed' then
+ iterative_calculation(1, 1, index) + iterative_calculation(1, 2, index)
end
end

Има още какво да се желае, но определено се вижда подобрение.

  • Главните забележки отново са относно имената:

Бих казал, че calculate беше по-добре от iterative_calculation. Утре трябва да си смениш изцяло имплементацията, без да променяш интерфейса. Кодът ти започва да лъже хората. xd Да, методът нямаше да е публичен (все пак ви искаме само series), но отново - името ти описва как решава някакъв проблем, а не какъв е самият проблем, който решава. Винаги като изчисляваш нещо итеративно можеше да го кръстиш iterative_calculation. Същото за iterator. Ясно е, че итерираш нещо, въпросът е - какво итерираш? Какво семантично значение имат числата от 2 до index?

first и second трябва да останат в имената на променливите, защото те си остават first и second за определен елемент от реда

Остават първото и второто какво? Не мисля, че term е точната дума (естествено е напълно възможно да има терминология, с която не съм наясно), опитай се да го преведеш.

Не се обезкуражавай, измислянето на добри имена е сред най-трудните проблеми в програмирането. Но също е и най-силният инструмент за писане на разбираем софтуер, независимо от език, технология и времеви период (или поне докато кодът ни все още е в текстова форма). (: Случва се да прекарвам ~ 30% от времето само в измислянето на имена. Понякога дори пренаписвам част от имплементацията си, само защото не съм успял да измисля добри имена за 2-3 променливи.

"There are only two hard problems in Computer Science: cache invalidation, naming things and off-by-one errors."

  • По принцип (не само в Ruby) тернарният оператор се използва за връщане на стойност, не просто като едноредов if. Когато има странични ефекти и не се интересуваш от резултата е малко изненадващо за читателя.
  • Може би липсва в style guide-а, но в Ruby се смята за добра практика да подравняваш някои повтарящи се на съседни редове конструкции. В това число са = и then.
  • Използвай then само за едноредови случаи.

Иначе skeptic не проверява абсолютно всичко. Той по-скоро служи, за да ви поставяме ограничения за дадената задача, отколкото да проверява за спазване на конвенции. Относно rubocop - нашият style guide е fork на community style guide-а от преди няколко години. Почти изцяло се припокриват, но има няколко разлики. Една от тях е точно индентацията на when. Ако изявяваш желание можеш да сравниш разликите и да contribute-неш към нашата версия, с екипа ще го обсъдим, най-вероятно ще получиш и точка.

  • За името на метода съм съгласен. За имената на index, first и second - не напълно. В условието на задачата сте описали тази променлива като индекс, така че се придържам към условието. Другите варианти за first и second, за които се сетих са previous и ante-previous (по-предишен), но мисля, че са неадекватни. Срещнах term в терминологията за редици и мисля, че се вписва. За мен е по-четимо така, при положение, че четящият е запознат с формулите.

  • Не разбрах какво точно виждаш като неправилен подход при тернарния оператор (и за кой тернарен оператор говориш).

Герасим обнови решението на 14.10.2014 14:25 (преди около 10 години)

-def iterative_calculation(first_term, second_term, index)
+def series_calculation(first_term, second_term, index)
return [first_term, second_term].max if index == 1
- 2.upto(index) do |iterator|
+ 3.upto(index) do |iterator|
iterator.even? ? first_term += second_term : second_term += first_term
end
index.even? ? first_term : second_term
end
def series(sequence_name, index)
case sequence_name
- when 'fibonacci' then iterative_calculation(0, 1, index)
- when 'lucas' then iterative_calculation(1, 2, index)
- when 'summed' then
- iterative_calculation(1, 1, index) + iterative_calculation(1, 2, index)
+ when 'fibonacci' then series_calculation(1, 1, index)
+ when 'lucas' then series_calculation(1, 2, index)
+ when 'summed'
+ series_calculation(1, 1, index) + series_calculation(1, 2, index)
end
end
x = condition ? a : b
condition ? x = a : x = b
  • Първото се смята за "правилно", второто за "неправилно". Тернарният оператор не е просто едноредов if. Когато го използваш имплицираш, че се интересуваш от връщаната стойност, а не от странични ефекти. На 4ти ред е точно обратното.
  • Както казах, възможно е да не знам терминология. Може ли да ми дадеш линк, към статия, която ги реферира като first_term и second_term?
  • Под това:

    Какво семантично значение имат числата от 2 до index?

Имах предвид iterator, не index.

  • Сега разбрах какво имаш предвид за тернарния оператор.
  • Прегледай тази статия. Добавям и дефиницията:
    • A sequence is an ordered list of numbers. The sum of the terms of a sequence is called a series.

Това е за term. Иначе, в статията можеш да видиш first term.

  • За iterator съм съгласен, че е прекалено общо название. Не съм сигурен дали вариантите, които ми идват наум, са по-смислени, но ще го променя. : )

Герасим обнови решението на 15.10.2014 12:21 (преди около 10 години)

def series_calculation(first_term, second_term, index)
return [first_term, second_term].max if index == 1
- 3.upto(index) do |iterator|
- iterator.even? ? first_term += second_term : second_term += first_term
+ 3.upto(index) do |series_step|
+ first_term, second_term =
+ sum_terms_over_series_step_parity(first_term, second_term, series_step)
end
index.even? ? first_term : second_term
+end
+
+def sum_terms_over_series_step_parity(first_term, second_term, series_step)
+ case series_step.even?
+ when true then first_term += second_term
+ when false then second_term += first_term
+ end
+ return first_term, second_term
end
def series(sequence_name, index)
case sequence_name
when 'fibonacci' then series_calculation(1, 1, index)
when 'lucas' then series_calculation(1, 2, index)
when 'summed'
series_calculation(1, 1, index) + series_calculation(1, 2, index)
end
end

Те са first_term и second_term на реда, който смятам във всеки един момент. Спрямо стъпката, те се променят. Нали F(n) = F(n-1) + F(n-2) - от това изхожда цялата функция. Идеята ми е да имплементирам функция, определяща F(n) в горния ѝ вид без значение от вида на подаваните аргументи, а не отделни функции за различните редици. Ще погледна решенията на този проблем на другите студенти, когато решенията станат публични, но в момента тези имена ми се струват най-адекватни.