Я создал катастрофу. Пожалуйста, укажите стратегию кодирования для рефакторинга

Question

UPDATE

Пока я значительно сократил это:

  # Gets the user's Wall
  # I used object methods so they can be called by other methods, such as #second_wall()
  def read_wall(fbuserid)
     @result ||= graph.get_connections(fbuserid, 'feed')
  end

  def second_wall(fbuserid)
     @second ||= @result.next_page
  end

  def third_wall(fbuserid)
     @third ||= @second.next_page
  end

  def fourth_wall(fbuserid)
       fourth ||= @third.next_page
   end

  # Collects your friends' wall Posts and puts the IDs into an array
  # the number array contains method names that will be called
  # This is done to read 100 wall posts
  def get_post_ids(fbuserid)
     var = []   
     number = [read_wall(fbuserid), second_wall(fbuserid), third_wall(fbuserid), fourth_wall(fbuserid)]
     number.each do |iterate|
        num ||= iterate
        for i in 0..25
           if find_nil(num, [i,'id']).nil? == false
              var << num[i]['id']
           end
        end
     end

Изначально у меня все было в базовом методе #read_wall (). Я не уверен, что случилось с массивом, но когда я попытался:

array = result + second + third + fourth. Я остался с данными только из оригинального результата. Так что я создал эту рабочую катастрофу. Не могли бы вы помочь мне с рефакторингом?

   # Gets the user's Wall
      def read_wall(fbuserid)
         result ||= graph.get_connections(fbuserid, 'feed')
      end

  def second_wall(fbuserid)
     result ||= graph.get_connections(fbuserid, 'feed')
     second ||= result.next_page
  end

  def third_wall(fbuserid)
       result ||= graph.get_connections(fbuserid, 'feed')
       second ||= result.next_page
       third ||= second.next_page
  end

  def fourth_wall(fbuserid)
        result ||= graph.get_connections(fbuserid, 'feed')
        second ||= result.next_page
        third ||= second.next_page
        fourth ||= third.next_page
   end

  # Collects your friends' wall Posts and puts the IDs into an array
  def get_post_ids(fbuserid)
     x ||= read_wall(fbuserid)
     var = []
     for i in 0..25
        if find_nil(x, [i,'id']).nil? == false
           var << x[i]['id']
        end
     end

     second_wall ||= second_wall(fbuserid)
       for i in 0..25
          if find_nil(second_wall, [i,'id']).nil? == false
             var << second_wall[i]['id']
          end
       end

      third_wall ||= third_wall(fbuserid)
        for i in 0..25
           if find_nil(third_wall, [i,'id']).nil? == false
              var << third_wall[i]['id']
           end
        end

      fourth_wall ||= fourth_wall(fbuserid)
         for i in 0..25
            if find_nil(fourth_wall, [i,'id']).nil? == false
                 var << fourth_wall[i]['id']
            end
         end

      @get_post_ids = var
  end

Answer 1

Добавление вспомогательной функции для каждой конкретной страницы в данном случае выглядит как запах кода. Это не всегда может быть проблемой, если вы очень часто говорите «1001» и вам нужен быстрый способ добраться до него. Однако, связывая каждую такую ​​функцию,

read_wall -> second_wall -> third_wall -> fourth_wall

это будет постепенно расширяться и поддерживать это. Может быть, есть базовая функция, которая может извлекать сообщения на стене с учетом любого пользователя и желаемого диапазона.

get_wall_posts(user_id, range)

Чтобы получить первые 100 сообщений на стене, и отображение идентификаторов будет выглядеть примерно так:

first_100_posts = get_wall_posts(some_user, 0...100);
first_100_ids = first_100_posts.map { |post| post['id'] }

Одна из наиболее важных частей рефакторинга - дать хорошие имена идентификаторам. Такие имена, как var и iterate, обычно мало что говорят о коде. Похоже, если не хуже, чем комментарии вроде

// increment i by 1
i++;

Последовательность так же важна. Группы методов, которые делают подобные вещи, должны быть названы непротиворечивым образом. read_wall и second_wall кажутся мне разъединенными, потому что глагол read отсутствует в других методах. read_second_wall звучит как лучшее имя и больше соответствует остальным.

Еще одна вещь, которую следует учитывать при проектировании, это согласованность объектов. В настоящее время некоторые из методов связаны процессом (получение первых 100 или 4 страниц постов), а не их индивидуальной ролью, что делает их зависимыми друг от друга. Вызов second_wall без вызова read_wall даст другие результаты, поскольку свойство result еще не было инициализировано. Это может привести к трудностям в поиске ошибок, потому что есть другое измерение, чтобы отслеживать - используются ли объекты в ожидаемом порядке.

Постарайтесь максимально ограничить побочные эффекты функций. Если возражение должно использоваться в определенном порядке, используйте шаблон state или что-то подобное, чтобы убедиться в этом.

Answer 2

В качестве подсказки, вместо повторения тела цикла четыре раза на четырех разных массивах, вы можете создать массив массивов.

Вместо этого:

for x in array1
  function1(x);
end

for x in array2
  function1(x);
end

вы можетесделать это:

[array1,array2].each do |array|
  for x in array 
    function1(x)
  end
end