Решение на Първа задача - температура и химични елементи от Исмаил Алиджиков

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

Към профила на Исмаил Алиджиков

Резултати

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

Код

class Substance
attr_reader :name
attr_reader :melting_point
attr_reader :boiling_point
def initialize(name, melting_point, boiling_point)
@name = name
@melting_point = melting_point
@boiling_point = boiling_point
end
end
SUBSTANCES = {
'water' => Substance.new('water', 0, 100),
'ethanol' => Substance.new('ethanol', -114, 78.37),
'gold' => Substance.new('gold', 1064, 2700),
'silver' => Substance.new('silver', 961.8, 2162),
'copper' => Substance.new('cooper', 1085, 2567)
}
def convert_between_temperature_units(degrees, from, to)
celsius = convert_to_celsius_from(degrees, from)
convert_from_celsius_to(celsius, to)
end
def convert_to_celsius_from(degrees, unit)
case unit
when 'C'
degrees
when 'F'
(degrees - 32) * 5 / 9.0
when 'K'
degrees - 273.15
else
raise ArgumentError.new("Unknown temperature unit #{unit}.")
end
end
def convert_from_celsius_to(degrees, unit)
case unit
when 'C'
degrees
when 'F'
(degrees * 9 / 5.0) + 32
when 'K'
degrees + 273.15
else
raise ArgumentError.new("Unknown temperature unit #{unit}.")
end
end
def melting_point_of_substance(substance, unit)
unless SUBSTANCES.key?(substance)
raise ArgumentError.new("Unknown substance #{substance}.")
end
convert_from_celsius_to(SUBSTANCES[substance].melting_point, unit)
end
def boiling_point_of_substance(substance, unit)
unless SUBSTANCES.key?(substance)
raise ArgumentError.new("Unknown substance #{substance}.")
end
convert_from_celsius_to(SUBSTANCES[substance].boiling_point, unit)
end

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

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

Finished in 0.00771 seconds
17 examples, 0 failures

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

Исмаил обнови решението на 13.10.2016 17:11 (преди около 8 години)

+class CelsiusConverter
+ def self.convert_to_unit(degrees, unit)
+ case unit
+ when 'C'
+ degrees
+ when 'F'
+ (degrees * 9 / 5.0) + 32
+ when 'K'
+ degrees + 273.15
+ else
+ raise ArgumentError.new("Unknown temperature unit #{unit}.")
+ end
+ end
+end
+
+class FahrenheitConverter
+ def self.convert_to_unit(degrees, unit)
+ case unit
+ when 'F'
+ degrees
+ when 'C'
+ (degrees - 32) * 5 / 9.0
+ when 'K'
+ (degrees + 459.67) * 5 / 9.0
+ else
+ raise ArgumentError.new("Unknown temperature unit #{unit}.")
+ end
+ end
+end
+
+class KelvinConverter
+ def self.convert_to_unit(degrees, unit)
+ case unit
+ when 'K'
+ degrees
+ when 'C'
+ degrees - 273.15
+ when 'F'
+ (degrees * 9 / 5.0) - 459.67
+ else
+ raise ArgumentError.new("Unknown temperature unit #{unit}.")
+ end
+ end
+end
+
+class Substance
+ attr_reader :name
+ attr_reader :melting_point
+ attr_reader :boiling_point
+
+ def initialize(name, melting_point, boiling_point)
+ @name = name
+ @melting_point = melting_point
+ @boiling_point = boiling_point
+ end
+end
+
+CONVERSION_LAMBDAS = {
+ 'C' => -> (d, u) { CelsiusConverter.convert_to_unit(d, u) },
+ 'F' => -> (d, u) { FahrenheitConverter.convert_to_unit(d, u) },
+ 'K' => -> (d, u) { KelvinConverter.convert_to_unit(d, u) }
+}
+
+SUBSTANCES = {
+ 'water' => Substance.new('water', 0, 100),
+ 'ethanol' => Substance.new('ethanol', -114, 78.37),
+ 'gold' => Substance.new('gold', 1064, 2700),
+ 'silver' => Substance.new('silver', 961.8, 2162),
+ 'copper' => Substance.new('cooper', 1085, 2567)
+}
+
+def convert_between_temperature_units(degrees, from, to)
+ unless CONVERSION_LAMBDAS.key?(from)
+ raise ArgumentError.new("Unknown temperature unit #{from}.")
+ end
+
+ CONVERSION_LAMBDAS[from].call(degrees, to)
+end
+
+def melting_point_of_substance(substance, unit)
+ unless SUBSTANCES.key?(substance)
+ raise ArgumentError.new("Unknown substance #{substance}.")
+ end
+
+ convert_between_temperature_units(SUBSTANCES[substance].melting_point, 'C', unit)
+end
+
+def boiling_point_of_substance(substance, unit)
+ unless SUBSTANCES.key?(substance)
+ raise ArgumentError.new("Unknown substance #{substance}.")
+ end
+
+ convert_between_temperature_units(SUBSTANCES[substance].boiling_point, 'C', unit)
+end

О, уау! Имам картинка точно за случая:

Колкото и добър инструмент да е един танк, ако трябва да убиеш муха, по-добре да използаш вестник.

Харесва ми, че си се запознал с класовете, ламбди, ексепшъни и т.н. Обаче задачата е изключително проста. А щом нещо е направено по-сложно, отколкото е необходимо - със сигурност тази сложност само ще пречи.

Всъщност, бих бил по-склонен да се съглася с ООП решение, ако твоето нямаше следните проблеми:

  • F, C и K се срещат на много места. Нали ООП трябваше да премахва дублиранията?
  • Ако трябва да добавиш нова единица, ще трябва да минеш през всички класове. Сега ще трябва да добавиш нов клас и да отидеш да едитнеш всички останали. Това не е добър ОО дизайн.
  • CONVERSION_LAMBDAS е нещото, което трябва да те подсети, че нещо не е наред със структурата ти. Имаш класове и въпреки това ти се налага да сложиш списък с нещата. Задачата е написана като за простото решение с хеш, затова сложното решение има нужда от адаптер към простия интерфейс на функциите. Тоест, това, което преминава от прости данни към класовете.
  • Substance. Защо имаш нужда от клас? С този клас симулираш прост хеш. 'water' => Substance.new('water', 0, 100) => 'water' => {melting_point: 0, boiling_point: 100}. По-кратко е (няма нужда от клас), спестяваш си дублиране на името на веществото, по-лесно се използва и е по-лесно за разбиране.

Още нещо. ArgumentError грешките се вписват в ООП решението ти. Няма да се вписват толкова добре в по-простото решение, плюс това сме казали, че няма да подаваме невалидни данни, така че спокойно можеш да изпуснеш тези проверки.

И един съвет. Не се хвърляй веднага да строиш йерархии и да прилагаш практики от Java. Почни от простото решение и шлайфай докато се получи добре.

Предполагам, че си стигнал до това решение като си се опитал да разделиш вложения if, който се получава при преобразуванията. Ако промениш малко логиката ще постигнеш много по-голямо подобрение - можеш просто да смяташ на две стъпки. Трябват ти единствено две функции - from_celsius и to_celsius. Тогава винаги минаваш през C. F -> K няма да има нужда да специфицираш как се смята, тъй като можеш да направиш F -> C -> K. Така като добавиш нова единица, вместо 6 нови случая ще имаш само 2.

Ех, пак изписах много. Какво мислиш за горните неща? Вероятно не си съгласен с нещо - кажи, нека го обсъдим.

Исмаил обнови решението на 16.10.2016 00:02 (преди около 8 години)

-class CelsiusConverter
- def self.convert_to_unit(degrees, unit)
- case unit
- when 'C'
- degrees
- when 'F'
- (degrees * 9 / 5.0) + 32
- when 'K'
- degrees + 273.15
- else
- raise ArgumentError.new("Unknown temperature unit #{unit}.")
- end
- end
-end
-
-class FahrenheitConverter
- def self.convert_to_unit(degrees, unit)
- case unit
- when 'F'
- degrees
- when 'C'
- (degrees - 32) * 5 / 9.0
- when 'K'
- (degrees + 459.67) * 5 / 9.0
- else
- raise ArgumentError.new("Unknown temperature unit #{unit}.")
- end
- end
-end
-
-class KelvinConverter
- def self.convert_to_unit(degrees, unit)
- case unit
- when 'K'
- degrees
- when 'C'
- degrees - 273.15
- when 'F'
- (degrees * 9 / 5.0) - 459.67
- else
- raise ArgumentError.new("Unknown temperature unit #{unit}.")
- end
- end
-end
-
class Substance
attr_reader :name
attr_reader :melting_point
attr_reader :boiling_point
def initialize(name, melting_point, boiling_point)
@name = name
@melting_point = melting_point
@boiling_point = boiling_point
end
end
-CONVERSION_LAMBDAS = {
- 'C' => -> (d, u) { CelsiusConverter.convert_to_unit(d, u) },
- 'F' => -> (d, u) { FahrenheitConverter.convert_to_unit(d, u) },
- 'K' => -> (d, u) { KelvinConverter.convert_to_unit(d, u) }
-}
-
SUBSTANCES = {
'water' => Substance.new('water', 0, 100),
'ethanol' => Substance.new('ethanol', -114, 78.37),
'gold' => Substance.new('gold', 1064, 2700),
'silver' => Substance.new('silver', 961.8, 2162),
'copper' => Substance.new('cooper', 1085, 2567)
}
def convert_between_temperature_units(degrees, from, to)
- unless CONVERSION_LAMBDAS.key?(from)
- raise ArgumentError.new("Unknown temperature unit #{from}.")
+ celsius = convert_to_celsius_from(degrees, from)
+ convert_from_celsius_to(celsius, to)
+end
+
+def convert_to_celsius_from(degrees, unit)
+ case unit
+ when 'C'
+ degrees
+ when 'F'
+ (degrees - 32) * 5 / 9.0
+ when 'K'
+ degrees - 273.15
+ else
+ raise ArgumentError.new("Unknown temperature unit #{unit}.")
end
+end
- CONVERSION_LAMBDAS[from].call(degrees, to)
+def convert_from_celsius_to(degrees, unit)
+ case unit
+ when 'C'
+ degrees
+ when 'F'
+ (degrees * 9 / 5.0) + 32
+ when 'K'
+ degrees + 273.15
+ else
+ raise ArgumentError.new("Unknown temperature unit #{unit}.")
+ end
end
def melting_point_of_substance(substance, unit)
unless SUBSTANCES.key?(substance)
raise ArgumentError.new("Unknown substance #{substance}.")
end
convert_between_temperature_units(SUBSTANCES[substance].melting_point, 'C', unit)
end
def boiling_point_of_substance(substance, unit)
unless SUBSTANCES.key?(substance)
raise ArgumentError.new("Unknown substance #{substance}.")
end
convert_between_temperature_units(SUBSTANCES[substance].boiling_point, 'C', unit)
-end
+end

Искам първо да ти благодаря за коментара. Наистина го оценявам!

  • F, C и K спокойно биха могли да бъдат изнесени в константи (евентуално като константи на модул, който съдържа класовете).
  • Съгласен съм, че ако добавиш нова единица, ще трябва да минеш през всички класове и да добавиш нов клас за единицата. Не разбирам защо не е добър дизайн при положение, че спазваш Open/Closed принципа.
  • Реших да създам клас Substance, понеже Substance.new('water', 0, 100) ми изглежда доста по-елегантно и self-explaining от {melting_point: 0, boiling_point: 100}. Освен това създаваш пет хеша с еднакви ключове. Това не би трябвало ли да алармира, че имаш duplication и че евентуално на някое място можеш да mistype-неш?
  • Да, знам, че подаваните данни нямат да бъдат невалидни. Понеже ползвам case, мисля, че е good practice да алармирам по някакъв начин за грешка в else клаузата, вместо нещо от сорта на puts 'Opsss'
  • Не смятам, че мисля на Java, докато пиша на Ruby. Мисля, че Ruby си има съвсем различни инструменти. Все пак ти може да дадеш по-обективна оценка.
  • Да, решението с допълнителното преобразуване в C е много по-просто. За целите на задача fit-ва много добре. Но ако си представиш, че в някой друг контекст преобразуванията ти са много скъпи би било безумно да избереш F -> C -> K пред F -> K. В такъв случай мисля, че би избрал подход, подобен на моя. Пак повтарям, за нашия случай, където сложността на операцията е константа, можем да си го позволим.

Като цяло ме изкефи, че си отделим време за да прегледаш решението ми. Благодаря ти отново! : )

Исмаил обнови решението на 16.10.2016 00:45 (преди около 8 години)

class Substance
attr_reader :name
attr_reader :melting_point
attr_reader :boiling_point
def initialize(name, melting_point, boiling_point)
@name = name
@melting_point = melting_point
@boiling_point = boiling_point
end
end
SUBSTANCES = {
'water' => Substance.new('water', 0, 100),
'ethanol' => Substance.new('ethanol', -114, 78.37),
'gold' => Substance.new('gold', 1064, 2700),
'silver' => Substance.new('silver', 961.8, 2162),
'copper' => Substance.new('cooper', 1085, 2567)
}
def convert_between_temperature_units(degrees, from, to)
celsius = convert_to_celsius_from(degrees, from)
convert_from_celsius_to(celsius, to)
end
def convert_to_celsius_from(degrees, unit)
case unit
when 'C'
degrees
when 'F'
(degrees - 32) * 5 / 9.0
when 'K'
degrees - 273.15
else
raise ArgumentError.new("Unknown temperature unit #{unit}.")
end
end
def convert_from_celsius_to(degrees, unit)
case unit
when 'C'
degrees
when 'F'
(degrees * 9 / 5.0) + 32
when 'K'
degrees + 273.15
else
raise ArgumentError.new("Unknown temperature unit #{unit}.")
end
end
def melting_point_of_substance(substance, unit)
unless SUBSTANCES.key?(substance)
raise ArgumentError.new("Unknown substance #{substance}.")
end
- convert_between_temperature_units(SUBSTANCES[substance].melting_point, 'C', unit)
+ convert_from_celsius_to(SUBSTANCES[substance].melting_point, unit)
end
def boiling_point_of_substance(substance, unit)
unless SUBSTANCES.key?(substance)
raise ArgumentError.new("Unknown substance #{substance}.")
end
- convert_between_temperature_units(SUBSTANCES[substance].boiling_point, 'C', unit)
+ convert_from_celsius_to(SUBSTANCES[substance].boiling_point, unit)
end

А, супер :) Радвам се, че се заформи дискусия :)

  • F, C и K спокойно биха могли да бъдат изнесени в константи (евентуално като константи на модул, който съдържа класовете).

Да, но реално само ще създадеш допълнителна индирекция - не решаваш проблем като създадеш константата C = 'C'. Не би променил стойността й, без да промениш името.

  • Съгласен съм, че ако добавиш нова единица, ще трябва да минеш през всички класове и да добавиш нов клас за единицата. Не разбирам защо не е добър дизайн при положение, че спазваш Open/Closed принципа.

Идеята на една добре организирана система е да е възможно да промениш минимална част от нея, за да я разшириш. Звучи логично и без да му даваме име и да го наричаме принцип. :) Open/closed е само едно от нещата, които са като насочващи подсказки. Съвсем не е единственото и не е достатъчно.

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

  • Реших да създам клас Substance, понеже Substance.new('water', 0, 100) ми изглежда доста по-елегантно и self-explaining от {melting_point: 0, boiling_point: 100}.

Да, разбирам, че Substance.new ти изглежда по-добре. В никакъв случай не твърдя, че е лошо решение. Искам да кажа, че има друга, по-проста опция, която в случая бих използвал аз. Въпрос на избор.

  • Освен това създаваш пет хеша с еднакви ключове. Това не би трябвало ли да алармира, че имаш duplication и че евентуално на някое място можеш да mistype-неш?

Създаваш пет инстанции на клас, плюс дефиницията на клас с присвояване на променливите. Повторение има и тук, тъй като пишеш Substance.new и повтаряш името на елемента дори без да ти трябва :) Представи си хеша като обект от анонимен клас - инстанцираш го с именовани аргументи. Каква би била разликата с Substance.new(melting_point: 0, boiling_point: 100) в случая? Също, говориш за дублиране на текст. Съгласен съм, че повторението на текст не е нещо добро, но пък е най-безопасния вид повторение. Особено ако смениш дублирането на ключове в данни със също толкова количество друг код.

Това кое ще използваш - клас или хеш - е въпрос на вкус. Аз бих избрал хешове за по-прости неща, а обектите - когато има по-сложни данни или трябва да се "ремикснат" по някакъв начин (например да се даде first_name и last_name, а да се вземе full_name).

  • Да, знам, че подаваните данни нямат да бъдат невалидни. Понеже ползвам case, мисля, че е good practice да алармирам по някакъв начин за грешка в else клаузата, вместо нещо от сорта на puts 'Opsss'

Да, добра идея е да имаш else на case-a винаги. Но това е по-скоро изключението от стандартната практика в Ruby. Например, често е по-добре да оставиш кода просто да гръмне вместо да проверяваш за невалидни аргументи.

  • Да, решението с допълнителното преобразуване в C е много по-просто. За целите на задача fit-ва много добре. Но ако си представиш, че в някой друг контекст преобразуванията ти са много скъпи би било безумно да избереш F -> C -> K пред F -> K. В такъв случай мисля, че би избрал подход, подобен на моя. Пак повтарям, за нашия случай, където сложността на операцията е константа, можем да си го позволим.

За всяко твърдение може да си представим случай, в който е неправилно :) Решаваме конкретен проблем - кодът, който пишем винаги е насочен към това. Ако можем да го напишем по-генерално без особени усилия - идеално. Обаче трябва да се стремим да не решаваме ненужно по-генерален случай за сметка на яснотата, на времето за написване на решението и на поддръжката после.
Дори сметката да е много голяма, пак бих измерил двата варианта и бих преценил по това кое е по-доброто. В повечето случаи е по-добре да напишеш код, който се поддържа по-лесно.

Здравей отново, : )

  • Да, но реално само ще създадеш допълнителна индирекция - не решаваш проблем като създадеш константата C = 'C'. Не би променил стойността й, без да промениш името.

Съгласен съм.

  • Това, което си представям в случая, е че за да добавя нова мерна единица просто ще трябва да добавя нов клас, или запис в хеш, или дори два реда в два case-а, стига да са на едно място. Не ми харесва идеята да трябва да модифицирам всички файлове.

Разбирам. Мислех, че е нуждата от това да модифицираш всички файлове при добавянето на нова мерна единица, е acceptable. Както вече дискутирахме, можем да избегнем това в контекста на нашата задача с много по-просто решение.

  • Да, разбирам, че Substance.new ти изглежда по-добре. В никакъв случай не твърдя, че е лошо решение. Искам да кажа, че има друга, по-проста опция, която в случая бих използвал аз. Въпрос на избор.

Имаме два варианта, които имат своите плюсове и минуси. Съгласен съм, че е въпрос на избор.

  • Представи си хеша като обект от анонимен клас - инстанцираш го с именовани аргументи. Каква би била разликата с Substance.new(melting_point: 0, boiling_point: 100) в случая?

Да, тази концепция за хеша като обект от анонимен клас е готина. Ако ползваме и keyword arguments двата варианта биха изглеждали съвсем еднакви. Въпрос на вкус, отново.

  • Да, добра идея е да имаш else на case-a винаги. Но това е по-скоро изключението от стандартната практика в Ruby. Например, често е по-добре да оставиш кода просто да гръмне вместо да проверяваш за невалидни аргументи.

Не е ли по-добра практика да проверяваш precondition-ити на даден метод и да се увериш, че работиш с валидирани данни, а ако не е така да хвърлиш подходящ exception?

  • Решаваме конкретен проблем - кодът, който пишем винаги е насочен към това. Ако можем да го напишем по-генерално без особени усилия - идеално. Обаче трябва да се стремим да не решаваме ненужно по-генерален случай за сметка на яснотата, на времето за написване на решението и на поддръжката после.

Съгласен съм с всичко, което си написал. Но ако си представиш, че става въпрос даден реален проект, всяка абстракция, която си вкарал, е в твой плюс. Трябва кодът да е написал така, че едва ли не трябва да си оставяш вратички за потенциални feature request-и в следващите тактове. Иначе с всеки нов task, ще ти се налага да рефакторираш кода си, за да го напаснеш към новите requirement-и. Съгласен съм, че едно от най-важните неща е кодът да бъде четим и да се поддържа лесно.