解析网页并将获取的图像保存到目录

require 'open-uri'
require 'nokogiri'

class Grab
    def runner(url, path)
        threads = []
        doc = Nokogiri::HTML(open("http://#{url}"))
        img_srcs = doc.css('img').map{ |i| i['src'] }.uniq
        img_srcs = rel_to_abs(url, img_srcs)
        img_srcs.each do |img_src|
            threads << Thread.new(img_src) do
                name = img_src.match(/^http:\/\/.*\/(.*)$/)[1]
                image = fetch img_src
                save(image, name, path)
            end
        end
        threads.each{ |thread| thread.join }
    end

    def fetch(img_src)
        puts "Fetching #{img_src}\n"
        image = open(img_src)
    end

    def save(image, name, path)
        File.open("#{path}/#{name}", "wb"){ |file| file.write(image.read) }
    end

    def rel_to_abs(url, img_srcs)
        img_srcs.each_with_index do |img_src, index|
          img_srcs[index] = "http://#{url}/#{img_src}" unless   img_src.match(/http:\/\//)
        end
        img_srcs
    end
end

#1 楼

几个小问题：



如果您支持带有协议（HTTP / HTTPS）的URL参数，那就太酷了：

doc = Nokogiri::HTML(open("http://#{url}"))

对于URL操作，您可以使用标准工具-URI :: HTTP / URI：HTTPS代替正则表达式：

name = img_src.match(/^http:\/\/.*\/(.*)$/)
img_srcs[index] = "http://#{url}/#{img_src}" unless   img_src.match(/http:\/\//)

此是一个不安全的操作，您应该转义名称：

File.open("#{path}/#{name}", "wb")

您忽略了HTTPS协议的链接：

img_src.match(/http:\/\//)

这不是Ruby方式：

img_srcs.each_with_index do |img_src, index|
  img_srcs[index] = "http://#{url}/#{img_src}" unless   img_src.match(/http:\/\//)
end
img_srcs

更简单一些

img_srcs.map do |src|
  src.match(/http:\/\//) ? "http://#{url}/#{src}" : src
end

您无法有效地加载页面：

image = open(img_src)

因此，对于创建的每个图像临时文件。这可能不是一种有效的方法。

open("http://stackoverflow.com/questions/1878891/how-to-load-a-web-page-and-search-for-a-word-in-ruby").class
=> Tempfile

#2 楼

海事组织（IMO），您花时间编写代码并且根本没有反馈是很不礼貌的。您的代码不错，但是总有改进的余地，提示：


使其更具功能性，
使用特定的库执行任务，
返回有用的东西对于主要方法（保存的文件路径？），
使用模块URI来处理url。

无论如何，只要有足够的时间并可以访问文档，那就是我要写的：

require 'nokogiri'
require 'typhoeus'

class Spider
  def self.download_images(url, destination_path, options = {})
    base_url = URI.join(url, "/").to_s
    body = Typhoeus::Request.get(url).body
    imgs = Nokogiri::HTML(body).css("img")
    image_srcs = imgs.map { |img| img["src"] }.compact.uniq

    hydra = Typhoeus::Hydra.new(:max_concurrency => options[:max_concurrency] || 50)
    image_paths = image_srcs.map do |image_src|
      image_url = URI.join(base_url, image_src).to_s
      path = File.join(destination_path, File.basename(image_url))
      request = Typhoeus::Request.new(image_url)
      request.on_complete { |response| File.write(path, response.body) }
      hydra.queue(request)
      path
    end
    hydra.run
    image_paths
  end
end

注意：具有相同文件名的图像将被覆盖，为简单起见，未考虑该图像。您只需使用group_by添加“ -n”后缀。

#3 楼

我对Ruby不熟悉，只是提示：如果我是对的，代码会为每个img标签创建一个线程，这可能太多了。例如，yahoo.com的源包含一百多个img标签，这意味着100多个线程和与同一主机的100多个连接。您应该将线程池与队列一起使用，以限制并行线程和并行连接的数量。

此外，您应该使用保持活动连接通过一个TCP连接下载多个映像。 （我不知道Ruby是否支持它，也不知道上面的代码是否使用它。）

#4 楼

不管怎么说，我的2c是代码实际上还不错，但是我认为这是一个基本的Web爬网程序，他们概述了性能一个关键目标，因此要解决的问题的一半将是提高缓存性能，即检测两次运行之间的缓存命中（还可以减少发生崩溃时从崩溃的运行中恢复的工作）。就像简单地正确组织下载并检查文件是否存在一样简单（尽管您需要警惕仅执行原子写入才能正常工作）。

总体而言，不聘用可能是多种原因，因为代码并不总是如此，所以不要太用力。

无论哪种方式，如果您主要是有关代码的反馈，对我来说标记的主要问题是：

没有错误处理

如果您基于代码而不是基于解决方案失败，我会下注缺陷，可能是由于缺少错误处理，或者如下所述，可能是有关x，y，z如何可能失败的评论。在这种情况下，您要处理线程，网络和文件IO，它们中的每一个都是潜在的炸药，并且如果要表现出色，应该进行错误处理。 （例如，如果某个线程引发异常，则可能会丢失所有工作，而不得不从头开始重新爬网：(）当然，根据面试过程，您可能已经对此类问题进行了测验，因此可能无关紧要。 。

缺少任何评论

根据您认为他们对您的判断有多苛刻以及代码的时间限制（面试，执行和发送），他们可能会期望您在代码中提供注释。尽管从我所见，这可能不是访谈中最大的问题。

#5 楼

我也不是ruby开发人员，但是在阅读您的代码后，这里有一些问题，

require 'open-uri'
require 'nokogiri'

class Grab
    def runner(url, path)
        threads = []
        doc = Nokogiri::HTML(open("http://#{url}"))

我假设您在这里得到一个数组

        img_srcs = doc.css('img').map{ |i| i['src'] }.uniq

rel_to_abs方法很奇怪。我认为您真正需要的是地图。（为什么要用这种方法解释）

        img_srcs = rel_to_abs(url, img_srcs)

为什么选择在外部执行rel_to_abs，但是在内部进行名称提取？

        img_srcs.each do |img_src|
            threads << Thread.new(img_src) do

名称提取失败时会发生什么？
您还假定提取的名称是有效的文件名。总是这样吗？ （如果是查询，会发生什么？）而且我更喜欢较小的匹配项，例如.*\/([^\/]+)$

                name = img_src.match(/^http:\/\/.*\/(.*)$/)[1]

您的访存实现感觉部分完成。我宁愿获取来读取图像或完全移动uri的开头进行保存。现在，资源（一个开放的连接）是通过一种方法获取的，而在另一种方法中使用的，而您本可以一口气做到这一点，并且避免了这种情况。 （我想这没什么错，但是感觉很不对。）

                image = fetch img_src
                save(image, name, path)

如果要由我决定，我会将整个名称处理并保存到一个save_to_disk(url,image-src,path)并在那里进行整个处理。它还具有不必将相对的mypath更改为http://domain/myapth格式，然后再次提取mypath的优势。在这里，我宁愿看到open(src).read这样开放的连接不会暴露在外面。

            end
        end
        threads.each{ |thread| thread.join }
    end

    def fetch(img_src)
        puts "Fetching #{img_src}\n"

这种特殊方法让人觉得很奇怪。您所需要的只是一张地图，用于检查img_src是否与http：//匹配，如果不匹配，则返回更新的版本。在这里，您正在foreach循环中更改数据结构（我同意这是可行的），而您没有理由这样做。

        image = open(img_src)
    end

    def save(image, name, path)
        File.open("#{path}/#{name}", "wb"){ |file| file.write(image.read) }
    end

还有什么原因让您更喜欢在Grab.runner中进行调用而不是在Grab.initialize中进行？对于后面的版本，您必须执行(Grab.new).runner(url,path)而不是Grab.new(url,path)

，但是这些都是次要的。基于这些我不会拒绝。所以我不知道。