Йоана обнови решението на 02.11.2014 01:26 (преди около 10 години)
Здравей :)
Браво, решението ти е доста добро! Парсването обаче не работи за един от примерите в условието на задачата.
Имам и някои идеи за подобрение :)
- Не е нужно да се използват регулярни изрази - всичко може да стане без тях и ще е по-ясен кода. Ако има конкретен случай, за който смяташ, че методът
split
няма да се справи - разгледай документацията му. Освен това[1..-1]
върху обекта, който връщаmatch
е много объркващо, поне за мен. - Присвояването на един ред в
Directory#initialize
не се използва често. Предпочита се да са си на отделни редове. Причината за това е, че ако станат повече променливите, ще трябва да ги разделиш или ще стане трудно за четене (ще трябва да се броят елементи). Пък и се създава един ненужен масив. - Няма нужда от проверката
if valid_name?
. Няма да тестваме с невалидни данни. (Иначе, ако трябваше да се проверява, по-добрият вариант би бил да се хвърли някакво изключение, вместо да се игнорира.) -
За условието не е нужно
add_file
иadd_directory
да връщатself
. Предполагам, че си го направила, за да можеш да си ги chain-ваш. Не казвам, че трябва да ги махнеш. Хубаво е, че си се замислила за това как ще се използва. :) Помисли и над този начин на използване, ако го нямашеself
:root = Directory.new rbfs = root.add_directory('rbfs') # Do something with `rbfs`
Това няма да го тестваме и можеш да си избереш как да го направиш, но винаги има плюсове и минуси - на единия и на другия подход.
- Харесва ми, че си се сетила за оператора
||
в#[]
.Directory#serialize
е доста хитро използване наmap
. - Хубаво е винаги на
join
да даваш разделителя експлицитно. Има глобална променлива ($,
), чрез която може да се задава разделителят по подразбиране и която може да счупи кода ти, ако е сетната на нещо различно от празен низ. - Chain-ването в
Directory.parse
ми е малко странно. Защо просто не си запазиш парсера в една променлива и да използваш нея? Не си спестила редове и няма да има нужда да връщашself
. Също така блоковете наparse_files
иparse_directories
е хубаво да ги направиш по един и същ начин, според мен ще е по-подредено. В случая - и двата сdo
. - Името на класа, който използваш за парсване е
DirectoryParser
, само че парсва и файлове. СамоParser
няма ли да е по-подходящо? - Метода
DirectoryParser#parse
може да го кръстиш напримерparse_list
илиparse_array
, така че да не се налага да му правиш синоними, за да изглежда по-четимо. Той реално не се интересува какви са нещата вътре. -
rest[0..length.to_i - 1]
може да се напише катоrest[0...length.to_i]
. - И последно - какво получаваш ако
serialize
е класов метод в модул, а не например private метод в Directory? Не го използваш на друго място.
ПП. Решението ти е по-кратко от моето. :)
Благодаря за забележките и най-вече, че ме подсети за split
:)
Относно методът serialize
- сметнах, че логиката трябва да се изнесе в отделен модул, но тъй като наистина не го ползвам на друго място, го направих private на Directory.