Марин обнови решението на 06.11.2011 00:46 (преди около 13 години)
Основно ме интересува, какво трябва да променя, за кода да в стил "Ruby Way". Защото, някак ми се струва, че не го правя по правилният начин и ще съм благодарен на коментар и критика.
Неща по синтаксиса и кода ти:
- Излишните неща в Ruby не са на почит; излишно е да се ползва
then
сif
, когато тялото наif
-а е на следващия ред - Класови (статични) методи в Ruby почти никога не се дефинират с
def SomeClass.my_method
; вместо това, за целта се ползва напримерdef self.my_method
- Когато декларираш или викаш метод, който няма никакви аргументи, избягвай да пишеш скобите; например
def build_delimiter()
е по-добре катоdef build_delimiter
- Спокойно може да ползваш
'999.99'.to_d
вместоBigDecimal('999.99')
- Конвенцията за константи, които не са имена на класове, е
MIN_PRICE
, а неMinPrice
- Скобите около условието в методи от типа на
Item#valid_price?
са излишни и е добре да се изпускат, например:MIN_PRICE <= price and price <= MAX_PRICE
- Изразите в някои
#discount_for
методи са леко сложни за тернари-оператора, ползавйif
-else
в такива случаи, за да улесниш четенето на кода - Не виждам къде си дефинирал
CartError
, но на едно място гоraise
-ваш - Когато дефинираш празни изключения, идиоматично е да ги събираш на един ред:
class MyError < RuntimeError; end
- На доста места бих оставял по един празен ред за яснота, напр. м/у дефинициите на методи/модули/класове; преди израза, връщан от даден метод, ако методът не е едноредов
- Хешът в
Discount.produce
е добре да изглежда така, ако ще го пишеш multiline:
{ get_one_free: DiscountGetOneFree, package: DiscountPackage, threshold: DiscountThreshold, }
Общи неща по дизайна ти:
- Като цяло ми изглежда малко твърде сложно като дизайн и с трудно-проследима логика
- Доста ми е странно да видя модул (че даже повече от един) в клас; обикновено класовете се намират в един или повече вложени модула, с цел създаване на именовано пространство (namespace), или пък модулите, предназначени за include-ване в други класове са си top-level (или в съответното именовано пространство на твоята библиотека, но вложени в друг клас — не)
- Бих кръстил класовете
CouponAmount
иCouponPercent
съответноPercentCoupon
иAmountCoupon
- Имаш страшно много
build_*
методи вInvoice
; повторение на част от името на метод също се брои за повторение :) може би нямаше да е лоша идея тези методи да бяха изведени в някакъв клас/модул, напримерReceiptBuilder
, който дефинира същите неща, но безbuild_
-префикс - Кодът ти ме умори :)
Благодаря за отделеното време. И съжалявам, че е бил толкова брутален кода... минах през няколко дизайна и накрая този стработи най-добре и ми изглеждаше готин (до момента). Естествено има поостанали неща, като build_
, когато смятах, че методите трябва да започват с действие, и класа се наричаше InvoiceBuilder
...
Като цяло if-then
са остатъци или са сложени да не крашва skeptic.
Иначе за съжаление, Руби няма готиния тернарен оператор на python true if cond else false
, или ако го има, аз не можах да го подкарам, за това направих най-лесното.
Ще се опитам да поизчистя малко кода, но вероятно архитектурата ще си я оставя същата.
Трябва някои лекции да се променят: http://fmi.ruby.bg/lectures/05-classes-namespaces-inheritance#24 Относно дефинирането на статични методи.
http://fmi.ruby.bg/lectures/02-arrays-hashes-functions#8 Относно именоването на константите
Относно лекциите, за класови методи още не е говорено в дълбочина и затова не е демонстрирана употребата на self
. Примерът от лекцията, който ти си дал, е само за "учебна" цел, преди да сме влезли в дълбочина и не отговаря на конвенцията, която се ползва на практика. Аз ти давам малко предварителни насоки :)
За константите, слайдът, към който си линкнал, си е технически правилен, няма нужда от промяна. Имената на класове също са константи и те се записват както ти ги ползваш — с CamelCase
. Но конвенцията за константи, които не са имена на класове, е CAMEL_CASE
.
Относно това за което питах в час: Ето горе-долу каква ми беше идеята.
class Inventory
def initialize
@items = {}
end
...
end
module Coupon
class Inventory
def initialize
super
@coupons = {}
end
def register_coupon
...
end
...
end
end
include Coupon
Т.е. правя си клас Inventory (или Cart..) и после след като инклудна модул, да разширя класа и по възможност да презапиша метода за инициализация...
Не стана... Защо? Прицнипно очаквах, че initialize няма да бъде override-нат, но очаквах и register_coupon да се появи в класа...
я пробвай с include Coupon
вътре в дефиницията на class Inventory
Тц... стана съвсем логичното:
metala@obsidian:~/projects/ruby-fmi$ pry -r ./test2.rb
[1] pry(main)> Inventory::Inventory
=> Coupon::Inventory
[1] pry(main)> i = Inventory.new
=> #<Inventory:0x000000025a6f20 @items={}>
[2] pry(main)> i.register_coupon
NoMethodError: undefined method `register_coupon' for #<Inventory:0x000000025a6f20 @items={}>
$ pry -r ./s.rb
pry(main)> Inventory::Inventory
(pry):1: warning: toplevel constant Inventory referenced by Inventory::Inventory
=> Inventory
pry(main)> Coupon::Inventory
=> Coupon::Inventory
pry(main)> Inventory.new.register # Мисля че е бъг във pry
Inventory.new.register Inventory.new.register_coupon Inventory.new.register_for
pry(main)> Inventory.new.register_coupon
NoMethodError: undefined method `register_coupon' for #<Inventory:0x000000019a11d0 @items={}>
from (pry):3:in `<main>'
pry(main)> Coupon::Inventory.new.register_coupon
"Works xD"
=> "Works xD"
pry(main)>
Предположение : Inventory и Coupon::Inventory са два различни Класа защото са във два различни ... namespace-а ^_^
аз всъщност имах предвид това : module Coupon def register_coupon p "Works xD" end end
class Inventory
def initialize
@items = {}
end
include Coupon #works
end
Това очевидно ще работи. Но това няма: module Coupon def initialize super @coupons = {} end def register_coupon p "Does not work X(" if not @coupons end end
class Inventory
def initialize
@items = {}
end
include Coupon
end
Но не това ми беше идеята.. Иначе предположението изглежда вярно, но все пак с include вкарвам ::Coupon::Inventory в ::
super
в initialize
на Inventory
и ... не се изкарва (предполагам че това е идеята)
pry(main)> Inventory.ancestors
=> [Inventory, Coupon, Object, PP::ObjectMixin, Kernel, BasicObject]
и не, вкарваш Coupon
във ::
, за Coupon::Inventory
не знам как става защото е клас а не module
:)
@Марин, Михаил е прав. Виж си ancestors
-веригата на Inventory
. Когато включиш модул, методите му идват по ред след методите на самия клас, т.е. той може да се разглежда като "родител" на Inventory
и следователно трябва да викаш super
в Inventory#initialize
, а не в Coupon::Inventory#initialize
.