Replace URL parameters with Ruby

Question

I have a method to replace URL parameters in an URL. It receives url as mandatory parameter and prefix and/or hash as optional parameters. Examples:

url_replace( '/news?b=2', { b: nil } ) # => '/news'
url_replace( '/news?b=2', { b: 3 } ) # => '/news?b=3'
url_replace( '/news?a=b', '/bar' ) # => '/bar?a=b'
url_replace( '/news?a=b&c=d', '/bar', c: nil ) # => '/bar?a=b'

The method:

def url_replace( target, *args )
 uri = URI.parse(URI.escape target)
 if hash = args.last.kind_of?(Hash) && args.last
 query = uri.query ? CGI.parse(uri.query) : {}
 hash.each do |k,v|
 v ? query[k.to_s] = v.to_s : query.delete(k.to_s)
 end
 uri.query = query.any? ? query.map{|k,v| "#{CGI.escape k.to_s}=#{CGI.escape Array(v).join}"}.join('&') : nil
 end
 prefix = args.first.kind_of?(String) && args.first
 uri.path = CGI.escape(prefix) if prefix
 CGI.unescape(uri.to_s)
end

I would like some refactoring or speed optimizations.

Okay, here's the code I ended up with:

def url_replace( target, *args )
 uri = URI.parse(URI::DEFAULT_PARSER.escape target)
 uri.path = CGI.escape(args.first) if args.first.kind_of?(String)
 if args.last.kind_of?(Hash)
 query = uri.query ? CGI.parse(uri.query) : {}
 args.last.each{ |k,v| v ? query[k.to_s] = v.to_s : query.delete(k.to_s) }
 uri.query = query.any? ? URI.encode_www_form(query) : nil
 end
 CGI.unescape(uri.to_s)
end

• tokland: out of curiosity: can you paste a URL that URI.parse does not like?
• ujifgc: URI.parse 'кококо'
• tokland: But URI is correct here, you have to escape it first (like any browser does): URI.parse(URI.escape('кококо')).
• ujifgc: Thank you, this makes sense. I updated my question with new code.

Answer 1

Some notes:

• url_replace( '/news' ): Each language has its formatting rules. In Ruby almost nobody inserts spaces after and before parens.
• hash = args.last.kind_of?(Hash) && args.last: I'd strongly discourage this kind of positional arguments, the method signature is severely impaired. Use an options hash instead (note that Ruby 2.0 finally provides keyword arguments).
• query.delete(k.to_s). If you check my other answers you'll see I tend to favour functional programming, so I'd rewrite this using expressions instead of statements. Code is much more clean when they say what things are instead of how you change their value.
• Uses of args.first in the middle of the code: Strive for declarative code, give names to things before you use them when it's not clear what they are.
• I'd accept only strings as keys for the query string, or, if Activesupport is at hand, I'd call stringify_keys or Hash[h.map { |k, v| [k.to_s, v] }] at some point. This way I'd avoid mixing symbols and strings.

I'd write:

require 'uri'
require 'cgi'
def url_replace(url, options = {})
 uri = URI.parse(URI.encode(url))
 hquery = CGI::parse(uri.query)
 components = Hash[uri.component.map { |key| [key, uri.send(key)] }]
 new_hquery = hquery.merge(options[:merge_query] || {}).select { |k, v| v }
 new_query = URI.encode_www_form(new_hquery)
 new_components = {path: options[:path] || uri.path, query: new_query}
 new_uri = URI::Generic.build(components.merge(new_components))
 URI.decode(new_uri.to_s)
end
puts url_replace('/news?a=b&c=d', path: '/bar', merge_query: {"c" => nil})
#=> /bar?a=b

• ujifgc: Thanks for the tips! I updated my code. I think, it's pretty straightforward and clean considering I like it imperative and tested.