"consuming" the hash instead of just fetching from it

B

Balint Erdi

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[:eek:nly_path]
url << (options[:protocol] || 'http')
url << '://' unless url.match("://")
...
end

Thank you,
Balint
 
A

Andrew Timberlake

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)
=A0 =A0 =A0 =A0url << (options.delete:)protocol) || 'http')
=A0 =A0 =A0 =A0url << '://' unless url.match("://")
=A0 =A0 =A0 =A0...
end

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

unless options[:eek:nly_path]
=A0 =A0 =A0 =A0url << (options[:protocol] || 'http')
=A0 =A0 =A0 =A0url << '://' unless url.match("://")
=A0 =A0 =A0 =A0...
end

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 Timberlake
http://ramblingsonrails.com

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

Robert Dober

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)
=A0 =A0 =A0 =A0url << (options.delete:)protocol) || 'http')
=A0 =A0 =A0 =A0url << '://' unless url.match("://")
=A0 =A0 =A0 =A0...
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
 
M

Markus Schirp

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

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

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

unless options[:eek:nly_path]
url << (options[:protocol] || 'http')
url << '://' unless url.match("://")
...
end

Thank you,
Balint

Markus
 
R

Robert Klemme

2009/5/22 Markus Schirp said:
This snippset let you test on "mispelled" or "extra" opts:

Example:

def initialize(opts)
=A0@a =3D opts.delete:)a) || raise("missing :a")
=A0@b =3D opts.delete:)b) || "b"
=A0raise "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

--=20
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,770
Messages
2,569,583
Members
45,072
Latest member
trafficcone

Latest Threads

Top