https://gist.github.com/AleksandarAleksandrov/49a7abaf7a6ef28259c2 Това е линка към по-доброто според мен решение за което стана дума в форума.

Здравей :)

Ето малко коментари по решението ти от gist-a:

• Някои методи плачат да използваш case. Напомням, че чрез case можеш да сравняваш класове на обекти, без да използваш is_a? и подобни методи.
• Когато имаш едноредов if го пиши по следния начин - <код> if <условие>, не if <условие> then <код> end.
• Защо имаш инстанционна променлива @content и get и set методи за data? Просто направи един attr_accessor :data и ползвай @data.
• Вместо да конкатенираш низове използвай интерполация. "test" + variable => "test#{variable}".
• Защо си използвал partition, където split би ти свършил по-добра работа? Виждам, че си ползвал и split надолу.
• По-удобно е да "разпакетираш" масива, който ти връща partition (или онзи по-подходящият метод), вместо да използваш arr[число].
• arr е ужасно име за променлива, също като Helper и Helper2.
• Имаш доста стилови проблеми:
  • На някои места си слагал скоби, на други - не.
  • Условията на if-овете не трябва да са оградени със скоби, а за извикването на методи - избери си един вариант и го следвай.
  • Предпочитаме and и or, вместо && и ||.
  • След запетаите между аргументите при извикване и дефиниране на метод се слага празно място.
• В Directory можеш доста да си улесниш методите, като съхраняваш файловете и директориите по малко по-различен начин - не чрез един хеш @content. Освен това, какво се случва ако имаш директория и файл с едно и също име? Това ще ти реши и проблема с нивата на влагане. Иначе в текущия вариант - files и directories можеш да ги напишеш малко по-функционално. Това с трупането на неща в обекта, който ще върнеш, е далеч от руби начина.
• И извън files и directories има по-добър начин от натрупването на неща в променлива като to_return. Сложи си стойностите в променливи и използвай интерполация, за да ги свържеш в общ низ, който ще върнеш.
• В Directory#parse правиш някакви сливания на хешове. Имаш attr_accessor - можеш директно да презапишеш съдържанието.

Класа Helper ще го коментирам отделно, тъй като според мен доста неща трябва да се преработят там. Започвайки от името му :)

• Направил си го само, за да заобиколиш ограничението за брой методи, нали?
• parse_contents:
  • Можеш да спестиш един ред, като махнеш присвояването file_count = file_count.to_i и го направиш на по-следващия ред file_count.to_i.times. Същото и за len.
  • Имена - имаш file_count, само дето метода може да чете и директории. len е лошо име, редом с arr и tmp. Не пишеш на C, опитай да изразяваш по-добре какво прави кода, чрез имената на променливите и методите.
• count няма да ти трябва, ако промениш начина, по който съхраняваш файловете и поддиректориите (това за @content). И все пак - има метод count в Enumerable.
• add_file_info и add_directory_info също ще се изпарят като следствие от горното.

Това бяха препоръки към текущото ти решение. :)

Понеже ти обещах да ти помогна с преструктурирането - ето някои по-абстрактни неща, над които да помислиш:

Добра идея е, когато имаш клас, да се стремиш максимално да го отделиш от другите. Тоест - да знае колкото се може по-малко за другите класове, особено за тяхната имплементация. Като тайните агенти - трябва да знаят само това, което им трябва, за да си свършат работата - не повече. На тази тема - parse_file_contents знае прекалено много за нещата, които парсва. Това може да го промениш, като например изкараш тази логика - directories ? RBFS::Directory.parse(data) : RBFS::File.parse(data) извън метода, използвайки yield. Тогава няма да ти се налага и да трупаш неща в entries, тъй като това ще го прави извикващият метода.

Друга добра идея е да пазиш текущото състояние в самия обект от конкретен клас. Тоест, вместо да връщаш rest, може просто в класа (който в момента се казва Helper) да има инстанционна променлива с текущия низ, която да променяш. Съответно методите вече няма да са класови.

Третата добра идея е един клас да не се занимава с прекалено много неща. Например Helper прави едновременно и парсване, и сериализация.

Ех, че голям ферман изписах. Надявам се да съм ти помогнал поне с част от препоръките ( и да не съм те демотивирал с количеството им :) ). Освен това, както сме казвали и на лекции, хубав код не се пише от веднъж. Винаги пишем нещо работещо, след това го променяме и променяме - докато сме доволни от резултата. В тази връзка очаквам нова ревизия, имаш още цяла седмица. :)

Мерси за изчерпателният отговор и препоръките. Със сигурност, ще ми помогнат да пооправя кода. Версията която качих тук и в git-а е първоначална, която гледах да работи, тъй като ми се очертават доста други домашни за седмицата и гледах да предам това домашно колкото се може по-рано. Все пак при първа възможно ще гледам да стегна малко кода :) . Отново мерси за бързия отговор.

Надявам се тази ревизия на домашното да е изпълнила по-голямата част от предишните забележки. Както и миналия път слагам линк към малко по-различното решение https://gist.github.com/AleksandarAleksandrov/102285b84945792f5c2b . Разликата в него е написването на метода parse за File използвайки case и премахването на Helper класа. Всякакви предложения за промяна са добре дошли :)

Малко по-добре е така, но все още има неща за оправяне:

• Тези защо са ти? Само ти запълват мястото за методи.

  def data
    @data
  end
  
  def data= (other)
    @data = other
  end
  
  def files
    @files
  end
  
  def directories
    @directories
  end
  

• Около запетаите, празни места се слагат само след тях. TrueClass , FalseClass -> TrueClass, FalseClass, count_files,count_directories,to_return -> count_files, count_directories, to_return и т.н.

• Не се слагат празни места между име на метод и скобите. initialize (data = nil) -> initialize(data = nil)
• Защо ти е този case във File.serialize? Не можеш ли да преизползваш някой от другите методи?
• В Ruby стойностите по подразбиране на аргументите могат да са всякакви изрази, включително и Directory.new - използвай го в add_directory.
• File.parse защо не е в case варианта си от gist-a? Ако те притеснява дължината на реда when 'number' ... може да помислиш как част от него да го изкараш в друг метод. Освен това има повторение на File.new, можеш ли да го направиш без повторението?
• Помисли какво ще се случи, ако имаш файл със съдържание, в което има :.
• #[] може да се напише и без проверката. Подсказка - hash[key] връща nil, ако key не съществува + определена логическа операция.
• За Directory#serialize:
  • Има две парчета код, които се повтарят и са почти едни и същи - отдели общата логика в един private метод и просто го извикай два пъти.
  • map и join е по-прегледно и ясно, от конкатенация на низове.
  • Можеш да си събереш резултатите в променливи и после да ги върнеш заедно чрез интерполация.
• За Directory.parse:
  • Отново има две почти еднакви парчета код. Същата процедура - отдели ги някъде в private метод.
  • Подредбата на кода е ужасна. Между другото end и празни редове не се зачитат от skeptic към ограничението.
• to_return е лошо име за променлива - не казва нищо полезно. Опитвай се, чрез името на променливата, да опишеш нейното съдържание и смисъл.