0

我最近申请了一份 Ruby 工作，并被要求编写一个方法来“编写一个名为“query_to_hash”的 Ruby 函数，该函数将单个字符串参数作为 HTTP 查询字符串（例如foo=bar&abc=1%202%203）并返回名称/值对的哈希，如下所示：

 {"foo" => "bar", "abc" => "1 2 3"}

我提供了附加的代码示例，并得到了一些反馈，表明我的 Ruby 风格不是他们想要的。

我现在很想知道开发人员会在附件中看到哪些风格问题，并希望得到建设性的反馈。

require 'rubygems'
require 'uri'
require 'rack'

include Rack::Utils

$SAFE = 3

# HTTP Query string (from wikipedia)

#field1=value1&field2=value2&field3=value3...
# The query string is composed of a series of field-value pairs.
# Within each pair, the field name and value are separated by an equals sign. The equals sign may be omitted if the value is an empty string.
# The series of pairs is separated by the ampersand, '&' (or semicolon, ';' for URLs embedded in HTML and not generated by a <form>...</form>; see below).
# While there is no definitive standard, most web frameworks allow multiple values to be associated with a single field[3][4]:

# field1=value1&field1=value2&field1=value3...

def query_to_hash(qry, sep = '&')

  # assume input string conforms to spec (no validation)
  # assume only & or ; is used - not both
  # return a null string if value is not defined
  # return null hash if query is null string
  # return array of values in hash if field has multiple values

  #@qry = qry.gsub(/%20/, " ")
  @qry = URI.unescape(qry)

  rtn = Hash.new {|h,k| h[k]=[]}

  if @qry == "" then 
  # return an empty hash
  #
    return {}
  else
      qry_a = @qry.split(sep)     
  end

  qry_a.each do |fv_pair|
    pair = fv_pair.split('=')

    # append multiple values if needed and ensure that null values are accommodated
    #
    rtn[pair[0]] << pair[1] ||= ""
  end 

  # collapse array if it contains only one item
  #
  rtn.each{|k,v| rtn[k] = *v if v.length == 1}

end



puts "Using 'query_to_hash' method:"
puts
test_values  = %w[foo=bar&abc=1%202%203 
                  foo&abc=1%202%203
                  foo=&abc=1%202%203
                  foo=bar&foo=boo&abc=1%202%203
                 ]

test_values.each { |v| puts "#{sprintf("%30s",v)} is returned as #{query_to_hash(v).inspect}" }           

test_values = %w[ foo=bar;foo=boo;abc=1%202%203
                  foo=bar;foo=boo;abc=1%0A2%203
                  foo=bar;foo=boo;abc=1%092%0D3
                 ]

test_values.each { |v| puts "#{sprintf("%30s",v)} is returned as #{query_to_hash(v, ';').inspect}" }       

puts "#{sprintf("%30s", "null string")} is returned as #{query_to_hash("").inspect}"

# compare with Rack::Utils::parse_query
#
puts
puts "Using 'Rack::Utils::parse_query' method:"
puts
test_values  = %w[foo=bar&abc=1%202%203 
                  foo&abc=1%202%203
                  foo=&abc=1%202%203
                  foo=bar&foo=boo&abc=1%202%203
                 ]

test_values.each { |v| puts "#{sprintf("%30s",v)} is returned as #{parse_query(v).inspect}" }           

test_values = %w[ foo=bar;foo=boo;abc=1%202%203
                  foo=bar;foo=boo;abc=1%0A2%203
                  foo=bar;foo=boo;abc=1%092%0D3
                 ]

test_values.each { |v| puts "#{sprintf("%30s",v)} is returned as #{parse_query(v, ';').inspect}" }       

puts "#{sprintf("%30s", "null string")} is returned as #{parse_query("").inspect}"
4

2 回答 2

1

我看不到任何对我尖叫“这是错误的”的东西。您的代码注释良好且非常清晰，并且您掌握了核心语言结构和基本 API。

是否给出了具体原因，还是他们只是使用了“我们不喜欢你的风格”？

免责声明：我是一名迁移到 ruby​​ 的 java 开发人员，所以我可能没有对正确的 ruby​​“风格”有最好的心理印象，但我可以说我们的系统中有更糟糕的代码。

于 2013-02-04T12:26:42.637 回答
1

只有两件事真正让我感到震惊。

qry_a = @qry.split(sep)

上一行中的变量是局部变量，只存在于else子句中，但您稍后会再次引用它。

@qry = URI.unescape(qry)

除非你有一个对象，否则不需要使用实例变量，我建议这是一个问题，因为它会立即打开方法之外的变量范围。就个人而言，我尽量使用当地人。

除此之外，我觉得还不错。也许您可以使用 Minitest 或 RSpec 之类的测试框架进行测试，但它的风格似乎不错。

我同意@mcfinnigan 的观点，更具体的观点会对您更有帮助。

我会补充一点（因为我有点仓促），直接使用数组索引也不是很好的风格，因为它很快就会变得混乱。例如，这看起来像 Perl 即线路噪音 :)

rtn[pair[0]] << pair[1] ||= ""

您还习惯于each在类似map或select可能更好的情况下影响值，因为它们显式地影响值而不是作为副作用。在 Ruby (IMO) 中，清晰明确是一种很好的风格。

再想一想……（我的咖啡开始了：）因为这是一次工作面试，也许他们正在寻找除此之外的东西。例如，我看到规范的第一个想法是“为什么将字符串作为参数传递？” 您可能会更好地执行以下操作：

class String
  def query_to_hash( separator="&" )
    # more code

然后可以从 调用它"foo=bar&abc=1%202%203".query_to_hash，这更像是Rubyish。

甚至比猴子补丁更好，继承（经常被忽视）。

class QueryString < String
  def to_hash( separator="&" )
    # some code that refers to `self`

然后可以通过以下方式使用它：

QueryString.new("foo=bar&abc=1%202%203").to_hash

这更易于阅读和理解，并使您想要实现的目标更加清晰。还有其他方法可以做到这一点，也许这就是让你与众不同的地方。

于 2013-02-04T12:33:32.887 回答