Рубиновый добытчик рекурсивно строится дерево


Это геттер в классе. Она рекурсивно генерирует дерево/хэш до определенного уровня глубины.

def children(depth: 4, article_children: self.root.child_links)
  get_children = lambda do |depth, article_children|
    article_children.map do |uri|
      if (depth == 0)
        Article.new(uri: uri)
      else
        article = Article.new(uri: uri)
        { article => get_children.call(depth - 1, article.child_links) }
      end
    end
  end

  @children ||= get_children.call(depth, self.root.child_links)
  return @children
end

Хотелось бы отзывы по этим конкретным направлениям:

  • Проблемы читаемость/ремонтопригодность
  • Это уместно использовать лямбда?
  • Проблемы стиля
  • Общей обратной связи


217
3
задан 12 апреля 2018 в 04:04 Источник Поделиться
Комментарии
1 ответ

Очевидно, что люди будут иметь разные мнения. Я думаю, что добавление лямда просто делает его более трудным для чтения, я бы поставил, что в частной emethod (которая также имеет то преимущество, что она для тестирования)

Есть ошибка в коде:

def children(depth: 4, article_children: self.root.child_links)
@children ||= get_children.call(depth, self.root.child_links)
return @children
end

Вы проходите self.root_child_links для get_children. Вы должны пройти мимо article_children.

Стиль заметки:

def children(depth: 4, article_children: self.root.child_links)
...
@children ||= get_children.call(depth, self.root.child_links)
return @children
end

В return как правило, считается плохим стилем. Достаточно указать только возвращаемое значение в последней строке. т. е. просто @children. В данном случае даже это не нужно, поскольку назначение возвращает значение @children. плохого я бы просто написал:

def children(depth: 4, article_children: self.root.child_links)
...
@children ||= get_children.call(depth, self.root.child_links)
end

Также (упростить этот кусок кода, чтобы показать мою точку зрения)

article_children.map do |uri|
article = Article.new(uri: uri)
{ article => get_children.call(depth - 1, article.child_links) }
end

Это возвращает массив хэшей, которая странным образом структурировать данные, не было смысла возвращать хэш.

Наконец, вместо того чтобы передать article_links в метод, который я хотел передать article.

Мой взгляд на этот код будет что-то вроде:

def children(depth: 4, root: self.root)
@children ||= recurse_children(depth, root)
end

private

def recurse_children(depth, article)
article.child_links.reduce do |hash, uri|
article = Article.new(uri: uri)
hash[article] = depth > 0 ? recurse_children(article) : nil
hash
end
end

2
ответ дан 12 апреля 2018 в 05:04 Источник Поделиться