Как я могу улучшить этот код ruby ​​и использовать хеши?

Дело здесь в том, чтобы просмотреть массив docfiles и вернуть два массива (temporary_file_paths а также temporary_file_names).

Я решил вернуть Hash, но я чувствую, что могу избавиться от 2-х временных массивов, но я не знаю, как...

   def self.foobar docfiles
        temporary_information = Hash.new
        temporary_file_paths = []
        temporary_file_names = [] 
        docfiles.each do |docfile|
          if File.exist? docfile.path
            temporary_file_paths << "new_path"
            temporary_file_names << "something_else"
          end
        end
        temporary_information[:file_paths] = temporary_file_paths
        temporary_information[:file_names] = temporary_file_names
        return temporary_information
    end

4 ответа

Решение

Здесь есть множество решений.

Возврат двойного значения.

def self.foobar(docfiles)
   temporary_file_paths = []
   temporary_file_names = [] 
   docfiles.each do |docfile|
     if File.exist? docfile.path
       temporary_file_paths << new_path
       temporary_file_names << something_else
     end
   end
   [temporary_file_paths, temporary_file_names]
end

paths, names = Class.foo(...)

Использование собирать.

def self.foobar(docfiles)
  docfiles.map do |docfile|
    File.exist?(docfile.path) ? [new_path, something_else] : nil
  end.compact
end

paths, names = Class.foo(...)

Использование inject (если вы хотите хеш)

def self.foobar(docfiles)
  docfiles.inject({ :file_paths => [], :file_names => []}) do |all, docfile|
    if File.exist?(docfile.path)
      all[:file_paths] << new_path
      all[:file_names] << something_else
    end
    all
  end
end

Все приведенные выше решения не меняют логику основного метода. Мне не очень нравится использовать массивы / хэши вместо объектов, поэтому я обычно заканчиваю преобразовывать хэши в объектах, когда выполнение требует нескольких разработок.

TemporaryFile = Struct.new(:path, :something_else)

def self.foobar docfiles
   docfiles.map do |docfile|
     if File.exist?(docfile.path)
       TemporaryFile.new(new_path, something_else)
     end
   end.compact
end

Кроме того, я не знаю значения something иначе, но если это то, что вы можете получить из new_path, тогда вы можете использовать ленивое выполнение.

TemporaryFile = Struct.new(:path) do
  def something_else
    # ...
  end
end

def self.foobar docfiles
   docfiles.map do |docfile|
     TemporaryFile.new(new_path) if File.exist?(docfile.path)
   end.compact
end

Да, просто используйте их вместо ваших временных хешей:

def self.foobar(docfiles)
    temporary_information = { :file_paths => [], :file_names => [] }
    docfiles.each do |docfile|
      if File.exist? docfile.path
        temporary_information[:file_paths] << new_path
        temporary_information[:file_names] << something_else
      end
    end
    return temporary_information
end

Вы можете избежать использования временных массивов, например, так:

def self.foobar docfiles
  temporary_information = {:file_paths => [], :file_names => []}
  docfiles.each do |docfile|
    if File.exist? docfile.path
      temporary_information[:file_paths] << new_path
      temporary_information[:file_names] << something_else
    end
  end
  return temporary_information
end

Вы также можете сделать этот шаг дальше и использовать inject:

def self.foobar docfiles
  docfiles.inject({:file_paths => [], :file_names => []}) do |temp_info,docfile|
    if File.exist? docfile.path
      temp_info[:file_paths] << new_path
      temp_info[:file_names] << something_else
      temp_info
    end
  end
end

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

Похоже, что вы можете найти решение неуклюже, но вот ваша упрощенная версия.

def self.foobar docfiles
     temporary_information = Hash.new
     temporary_information[:file_paths] = []
     temporary_information[:file_names] = []

     docfiles.each do |docfile|
       if File.exist? docfile.path
         temporary_information[:file_paths] << new_path
         temporary_information[:file_names] << something_else
       end
     end

     return temporary_information
 end

Также, как предполагает Алекс, вы можете использовать inject, Ниже приведен рабочий пример, чтобы показать вам сокращенную версию, которую я написал до того, как увидел сообщение Алекса:-)

['test', 'test2', 'test3'].inject({}) { |result,element| { :file_paths => result[:file_paths].to_s + element, :file_names => result[:file_names].to_s + element } }
Другие вопросы по тегам