"consuming" the hash instead of just fetching from it

Hi,

Deleting (vs. just fetching) the hash key from an options hash that
was passed in as an argument to the method seems prevalent. I saw it
in several high-quality OS projects (e.g DataMapper, Rails)

For example:

unless options.delete(:only_path)
url << (options.delete(:protocol) || ‘http’)
url << ‘://’ unless url.match("://")

end

I wonder what advantage the above snippet has instead of writing:

unless options[:only_path]
url << (options[:protocol] || ‘http’)
url << ‘://’ unless url.match("://")

end

Thank you,
Balint

On Fri, May 22, 2009 at 1:10 PM, Balint E. [email protected]
wrote:

   url << '://' unless url.match("://")

Thank you,
Balint

It’s usually a good idea to consume the option if you are receiving
options that any methods you will subsequently call, won’t need (or
may break when receiving) them.
i.e. options specific for the particular method.

If you are not going to pass the options onto other methods, then
there is nothing wrong with just referencing them.

Andrew T.
http://ramblingsonrails.com

http://MyMvelope.com - The SIMPLE way to manage your savings

On Fri, May 22, 2009 at 1:10 PM, Balint E. [email protected]
wrote:

Hi,

Deleting (vs. just fetching) the hash key from an options hash that
was passed in as an argument to the method seems prevalent.
I strongly disagree.
I saw it
in several high-quality OS projects (e.g DataMapper, Rails)
Rails high-quality? On what basis?
Please do not take this as a provocation, it is just the first time I
have heard this claim (well maybe DHH said something similar once, but
do not quote me, please;)

For example:

unless options.delete(:only_path)
url << (options.delete(:protocol) || ‘http’)
url << ‘://’ unless url.match(“://”)

end

I wonder what advantage the above snippet has instead of writing:
I would say that context is king. If the author of the above snippet
deletes values from the hash she has probably a (good) reason.
It is not very difficult to imagine some reasons, e.g. checking later
for the same values again (but please see below).
I would however take another look at the whole setup of the thing, if
I were e.g. a code reviewer. This seems not a case I would mute
objects a priori, thus I too would be intrigued by the rationale
behind this.
If you are too, which I have understood from your post, than you might
just want to follow the lifetime of options in the code to
understand/approve/disapprove about this code.

HTH
Robert

2009/5/22 Markus S. [email protected]:

    url << (options.delete(:protocol) || 'http')

@b = opts.delete(:b) || “b”
raise “unkown options: %s” % opts.keys.inspect unless opts.empty?
end

You can have that with the non deletion approach as well.

I agree to what Robert said: this should be carefully inspected.
Generally I would refrain from changing an argument, especially if it
is some global options object. These options might be needed in other
places as well and if they are modified by code other than the option
parsing (or reading) process then chances are that something will
break. For example, consider two places in code which need to react
on the same option value then only one of them gets the value with the
“delete pattern” and even worse, things may accidentally work
initially but break if initialization order is changed.

Also, if the options instance is frozen after being filled by option
parsing or reading process the delete approach won’t work any more.
As a general rule of thumb it is safer to not modify option arguments.

Kind regards

robert

On Fri, May 22, 2009 at 08:10:09PM +0900, Balint E. wrote:

    url << '://' unless url.match("://")
    ...

end

This snippset let you test on “mispelled” or “extra” opts:

Example:

def initialize(opts)
@a = opts.delete(:a) || raise(“missing :a”)
@b = opts.delete(:b) || “b”
raise “unkown options: %s” % opts.keys.inspect unless opts.empty?
end

Balint
Markus