Хешове и keyword аргументи на блокове

Програмиране с Ruby

Курс във Факултета по Математика и Информатика към СУ

Решение на Първа задача от Огнян Ангелов

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

Към профила на Огнян Ангелов

Резултати

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

Код

def convert_to_bgn(amount, currency)
currencies = {bgn: 1, usd: 1.7408, eur: 1.9557, gbp: 2.6415}
(amount * currencies[currency]).round(2)
end
def compare_prices(amount, currency, other_amount, other_currency)
amount_in_bgn = convert_to_bgn(amount, currency)
other_amount_in_bgn = convert_to_bgn(other_amount, other_currency)
amount_in_bgn - other_amount_in_bgn
end

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

........

Finished in 0.00733 seconds
8 examples, 0 failures

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

Огнян обнови решението на 10.10.2015 13:36 (преди над 8 години)

+
+def convert_to_bgn(val, currency)
+ currencies = {:bgn => 1, :usd => 1.7408, :eur => 1.9557, :gbp => 2.6415}
+
+ val * currencies[currency]
+end
+
+
+def compare_prices(val, currency, compared_val, compared_currency)
+ converted_val = convert_to_bgn(val, currency)
+ converted_compared_val = convert_to_bgn(compared_val, compared_currency)
+
+ converted_val - converted_compared_val
+end

Огнян обнови решението на 10.10.2015 13:41 (преди над 8 години)

-
def convert_to_bgn(val, currency)
currencies = {:bgn => 1, :usd => 1.7408, :eur => 1.9557, :gbp => 2.6415}
- val * currencies[currency]
+ (val * currencies[currency]).round(2)
end
def compare_prices(val, currency, compared_val, compared_currency)
converted_val = convert_to_bgn(val, currency)
converted_compared_val = convert_to_bgn(compared_val, compared_currency)
converted_val - converted_compared_val
end

Здравей :)

Добро решение! Имам няколко препоръки:

  • Харесва ми, че си разгледал и използвал хеш. В Ruby има по-кратък запис за хешове с ключове, които са символи: {bgn: 1, usd: 1.7408, ...} - препоръчително е да се използва той, когато е възможно.
  • Не ми допада името на променливата val. За това има две причини:
    • Какво е val? Защо не value? Два символа толкова ли ще спестят и дали си заслужава? Ако да, защо няма curr вместо currency? curr ще спести 4 букви :) Може направо c. Къде е границата? :)
    • value означава стойност. Добре, променливата съдържа стойност. Каква стойност? Всяка една променлива може да се кръсти value и името ѝ няма да дава никаква информация за съдържанието. Според мен price ще бъде по-ясно описание. :)
  • В compare_prices втората част от променливите започват с compared. Това означава, че те се сравняват. Първите нямат compared. Означава ли това, че не се сравняват? :) Това дали се сравняват не е нещо разграничаващо за двете двойки аргументи. Може да ги разграничиш по друг начин - например first_price и second_price. Сравняваш първата цена с втората.
  • converted също не е особено описателно. Конвертирано какво? От какво? В какво? price_in_bgn би отговорило на тези въпроси.

Тези коментари може да ти се сторят дребнави, но имената на променливи и функции са едно от най-важните неща за четимостта на кода. Стреми се да измисляш максимално описателни имена, за да не се налага хората, които четат кода да си задават въпроси, чиито отговор трябва да намерят ровейки из кода. За тази задача е очевидно какво се случва дори имената да са a, b, c и т.н., но в един малко по-сложен код няма да е така.

Напомням, че все още имаш възможност да подобриш кода и да ни изпратиш ново решение :) И ако не си съгласен с някой от коментарите сподели, за да го дискутираме. Целта не е да се съгласиш безусловно, а в дискусия да се зароди истината. :)

Огнян обнови решението на 10.10.2015 16:45 (преди над 8 години)

-def convert_to_bgn(val, currency)
- currencies = {:bgn => 1, :usd => 1.7408, :eur => 1.9557, :gbp => 2.6415}
+def convert_to_bgn(amount, currency)
+ currencies = {bgn: 1, usd: 1.7408, eur: 1.9557, gbp: 2.6415}
- (val * currencies[currency]).round(2)
+ (amount * currencies[currency]).round(2)
end
-def compare_prices(val, currency, compared_val, compared_currency)
- converted_val = convert_to_bgn(val, currency)
- converted_compared_val = convert_to_bgn(compared_val, compared_currency)
+def compare_prices(amount, currency, compared_amount, compared_currency)
+ amount_in_bgn = convert_to_bgn(amount, currency)
+ compared_amount_in_bgn = convert_to_bgn(compared_amount, compared_currency)
- converted_val - converted_compared_val
-end
+ amount_in_bgn - compared_amount_in_bgn
+end

Привет,

Благодаря за коментара. Не знаех за краткия синтаксис на хешовете. Съгласен съм с аргументите срещу "val" също и за "converted".

Не съм сигурен относно префикса на аргументите - "compared". Не виждам как имена като "first_x", "second_x", "x1", "x2", etc. са ясни и спомагат за четимостта на кода (Първоначално така ги бях кръстил). Идеята на compared е, че ние сравняваме две суми една спрямо друга (amount compared to another amount). Не ми звучи добре compared_to_amount за това реших да го оставя така.

Поздрави

Супер, така е доста по-добре :)

Логиката ми за compared е, че това е като някаква характеристика, която я даваш на втората цена - тя се сравнява. Но не я даваш на първата. Това за мен означава, че първата цена не се сравнява. Реално и двете цени са compared, защото се сравняват една с друга. Тоест трябва да ги различим не по тяхна характеристика, а по нещо друго.

  • first_price, second_price = "Сравнявам първата цена с втората цена."
  • price_one, price_two = "Сравнявам цена едно с цена две."
  • price, compared_price = "Сравнявам цената със сравнената цена."
  • price, another_price = "Сравнявам цената с другата цена."

Така ги разбирам аз.

Огнян обнови решението на 10.10.2015 23:08 (преди над 8 години)

def convert_to_bgn(amount, currency)
currencies = {bgn: 1, usd: 1.7408, eur: 1.9557, gbp: 2.6415}
(amount * currencies[currency]).round(2)
end
-def compare_prices(amount, currency, compared_amount, compared_currency)
+def compare_prices(amount, currency, other_amount, other_currency)
amount_in_bgn = convert_to_bgn(amount, currency)
- compared_amount_in_bgn = convert_to_bgn(compared_amount, compared_currency)
+ other_amount_in_bgn = convert_to_bgn(other_amount, other_currency)
- amount_in_bgn - compared_amount_in_bgn
-end
+ amount_in_bgn - other_amount_in_bgn
+end