url_expander icon indicating copy to clipboard operation
url_expander copied to clipboard

UrlExpander::Client.expand changes its options argument

Open korny opened this issue 12 years ago • 0 comments

A method should not change its parameters; this is generally seen as bad style.

It produced weird behavior for me because I was using a constant as the options:

        UrlExpander::Client.expand(url, URL_EXPANDER_OPTIONS)

Unfortunately, if you have nested short URLs (even just 2 levels), this means that inside the expand method, options[:limit] (starting at 10) is decreased by 1 on each call, finally resulting in an ArgumentError from this line:

      raise ArgumentError, 'HTTP redirect too deep' if options[:nested_shortening] && options[:limit] == 0

As mentioned, I think the bug here is that the options parameter should never be changed, but duplicated if it needs to be mutable internally.

korny avatar Feb 13 '13 15:02 korny