patternrubyMinor
Replace URL parameters with Ruby
Viewed 0 times
withreplacerubyparametersurl
Problem
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:
The method:
I would like some refactoring or speed optimizations.
Okay, here's the code I ended up with:
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)
endI 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)
endSolution
Some notes:
I'd write:
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.firstin 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_keysorHash[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=bCode Snippets
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=bContext
StackExchange Code Review Q#28917, answer score: 4
Revisions (0)
No revisions yet.