Екатерина обнови решението на 31.10.2014 16:18 (преди около 10 години)
Малко не особено незадълбочени коментари по стил и дреболии из кода и съвсем малко "архитектурни" забележки:
- Каква е причината да monkey patch-ваш
TrueClass
иFalseClass
, като включвашBoolean
вътре? Не виждам къде го ползваш това. А и е лоша идея. Самият модул не е лоша идея. Ако се monkey patch-ва, може да се направиString#to_b
илиString#to_boolean
, който прави това. Така ще може да ползваш'true'.to_b
. Това понякога се прави, но е глезотия и е по-добре да се избягва. - Самия метод аз бих го кръстил
from_string
. Тъй като е модул-метод и ще го викаш само с модула, ще се ползва много ясно, ето така:Boolean.from_string('true')
. Ето това е хубав начин да имаш такава функционалност. - Същите бележки и за
Number#create_number_from_string
-
get_data_type
си плаче за@data_type = case @data ...
. Ако това е междинен вариант на кода, окей. Иначе не виждам никакъв смисъл от всички тези еднакви изрази там. - Името на този метод е лошо. Аз бих го кръстил
infer_data_type
,set_data_type
или нещо такова. Той не get-ва, а set-ва. - Интересен факт е, че интерполацията в Ruby автоматично вика
to_s
на обектите. Тоест,serialize
може да се напише така:"#{@data_type}:#{@data}"
. Даже и така, но аз не харесвам това изписване: "#@data_type:#@data". -
Когато ключовете ти в речник са само символи, ползвай съкратения синтаксис (редове 32-36), т.е.:
TYPES = { nil: NilClass, number: Number, ... }
И оставяй запетайка след последния елемент в речници и списъци, които си дефинирала на повече от един ред.
Отмествай тялото на
case
едно ниво навътре (редове 72-75).- Нов материал: за да дефинираш класови/модул методи, може да ползваш и
def self.foo() ... end
. Тоест, вмето името на класа или на модула, ползвашself
. Това се предпочита по обясними причини :) - Паралелно присвояване няма нужда да се ползва в случаи като този на ред 117. По-добре разпиши присвояването на два реда. По-просто и по-четимо.
-
serialize_files
бих го написал като@files.map { |file_info| serialize_unit(*file_info) }.join('')
. Същото и за директориите. - Хубаво е да има празен ред между 113 и 114.
- На ред 18 скобите не са сложени правилно; трябва да са около аргумента на
include?
, т.е.string.include?('.') ? ... : ...
.
Нещо не ми харесват нещата в DirectorySerializer
модула. Имената на променливите, кодът, гъстотата на символите... Мисля, че може да се пренапише. Обърква ме и публичния му интерфейс. Има един инстанционен метод и няколко модул-метода. Защо не го направиш на клас? Инстанцирай си го, ползвай му публичните и private методите и така. Инстанционният метод просто го сложи в Directory
, не го преизползваш другаде, като гледам. Аз бих пробвал пренаписване на тази част от кода, с DirectorySerializer
-а.