Как я могу улучшить этот код 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 } }