Георги обнови решението на 15.12.2013 23:57 (преди почти 11 години)
Бързи бележки:
- Интересно DRY-ване на проблема със сравнението; аз бих пробвал да капсулирам тези помощни модули в общия namespace
- За
==
може да ползваш синоним на метод, няма да има проблем - Няма експлицитно изискване да може да сравняваш геометричните обекти за по-малки и по-големи, изисква се само равенство; не казвам, че трябва да го махнеш, просто че не го изискваме
- Като правиш библиотека, която други хора ще ползват, трябва да си тройно по-въздържан да ползваш метапрограмиране; пробвай да запазиш решението си за сравнение, но без метапрограмиране – например с plain-old-Ruby-include
- Като алтернатива на горното, ако много държиш на метапрограмирането, поне дефинирай клас-макрото в
Class
, а не вObject
- Махни празните редове 35, 80, 277; на тези места не се оставят такива според нашите конвенции
- Смяташ ли, че тази имплементация на вътрешно представяне на пано е най-оптимална? Струва ми се, че има и по-добри варианти
- Трябва да има празен ред м/у 67 и 68, както и м/у 267 и 268 (интересно, точно 200 реда разлика)
- Бих сложил запетая в края на 84-ти ред
- Ред 211,
step_count.succ.times do ...
- Замразяваш пискелите в
Ascii
, но не и символа за нов ред на 87? - Нещо не ме кефи това
canvas.coordinates(true)
; не ми е достатъчно ясно какво прави товаtrue
, трябва да ходя и да гледам дефиницията на метода; ако държиш да е такъв интерфейса, поне пробвай нещо друго, например keyword arguments:canvas.coordinates(yield_new_line: true)
– далеч по-ясно е; този аргументyield_new_line
ми изглежда като някаква хакерия, подсказваща за нужда от по-различен дизайн - Като цяло, не разбирам защо рендерерите не ползват съществуващия интерфейс на паното, ами си дефинираш нов интерфейс за целта, който "вади" навън нещо много близко до вътрешното представяне на паното; при съществуващия ти интерфейс какво правиш, ако рендерерът е такъв, че се налага да чертае линиите отдолу нагоре, или пък отдясно наляво, или пък първо нечетните, после четните редове? Не трябва ли рендерерът да реши как да ползва паното? Не те задължавам да си променяш дизайна, просто разсъждавам на глас
- Ако фрийзвам низове, бих го направил консистентно навсякъде, в т.ч. и в константите на
Html
; също така, там бих сложилHEADER
иFOOTER
един до друг; тук ще ти хинтна, че при мен има само една константа, която еTEMPLATE
:) - Когато ползваш
join
, добра практика е да подаваш винаги, експлицитно с каквоjoin
-ваш; не разчитай на дифолта, той се контролира от шантава глобална променлива - Интересно, дефинирал си аритметика на точки, за да я ползваш в растеризацията на линия; хм, дали е достатъчно основание това? Просто въпрос.
- Интересно също така, че имаш методи
draw_on
иrasterize_on
; на мен втория ми харесва повече като име - Нещо ме дразни в методите за ъгли на правоъгълника; не съм категоричен, но все пак виж дали няма да успееш да ги рефакторираш по-нататък някак
Добро решение! :)
Моето е 297 реда. Мисля, че бих могъл да разкарам някои неща там. Основното тегаво нещо при мен е растеризацията на линия. При теб изглежда доста по-добре :)
Бързи-бързи... Колко да са бързи.
Благодаря за коментарите!
- За метапрограмирането се учудих, че няма нещо такова в стандартната библиотека предвид колко много неща има там, стори ми се като нещо, което би се използвало често.
- Сравнението го използвам при точките, за да мога да кажа това:
from, to = to, from if from > to
- За фрийзването общо взето се чудих доста дали има смисъл да го правя над константи, които са стрингове. Те ще се преизползват така или иначе. За промяна - ако някой ги промени си е на негова отговорност. Как е най-добре и какви са доводите за конкретен избор? Специално за хеша го фрийзвам, за да не се добавят други ключове, които така или иначе няма да се използват. Пикселите в Ascii съм забравил да ги "размразя".
- Да,
canvas.coordinates(true)
и мен не ме кефи особено, но ударих в шестте реда на метод и това е първото, за което се сетих. Иначе рендерерите пак могат да си ползватpixel_at?
. Ще го направя по друг начин. - За
TEMPLATE
не знам как не се сетих :) - Аритметиката на точки - не е достатъчно основание, обаче не ми се стори по-прегледно да създавам нова точка ръчно в растеризирането, или да пазя отделни променливи за
x
иy
(again, 6 реда на метод). :) Знам, че мога да разбия растеризирането на още методи, но не ми се струва уместно. А може и да не съм се сетил за по-добър вариант още, има време до неделя :) -
draw_on
иrasterize_on
<= 6 реда на метод :) - Какво точно имаш предвид за ъглите на правоъгълника? Можех да ги изчисля в началото и да ги запазя... Всъщност в такива случай, когато няма публични мутиращи функции, да разчитам ли на това, че някой няма да ми промени инстанционните променливи и да си сметна нещата, които зависят само от тях предварително?
- Относно замразяването, аз предпочитам да замразявам неща, присвоени на константи, за да може ако някой неволно ги промени (дори не директно, а променяйки променлива, на която е била присвоена стойността от константата), да получи изключение рано (fail early)
- Растеризирането ти на линия е интересно, може и така да остане. Аз просто си разсъждавах на глас. Но пък ако ти дойде нова идея преди крайния срок, ще е интересно да я видим :)
-
Относно ъглите на правоъгълника (и не само), разбира се, че трябва да разчиташ на това, че няма някой да ползва
instance_variable_set
; това вече е ясно прекрачване на бариерата и определено се прави на отговорността на прекрачващия; за константите не е точно така, защото не е далеч от ума следният хипотетичен сценарий:HOSTNAME = 'foo.bar.com' if valid_tld? HOSTNAME connect_to HOSTNAME ... end def valid_tld?(host) tld = host.slice!(-3..-1) %w( com net org ).include? tld end
Допълнение: За ъглите на правоъгълника, пробвай да го направиш без if
-ове в методите. Виж дали ще стане и дали ще ти хареса, ако стане.