Марин обнови решението на 06.11.2011 00:46 (преди около 14 години)
Основно ме интересува, какво трябва да променя, за кода да в стил "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.
