On Mon, Dec 10, 2012 at 8:44 PM, Intransition <[email protected]> wrote:
>
>
> On Monday, December 10, 2012 7:22:17 AM UTC-5, Robert Klemme wrote:
>>
>> On Sun, Dec 9, 2012 at 7:19 PM, Intransition <[email protected]> wrote:
>> > The note `TODO: Improve Hash#rekey code!!!` has been in my docs for too
>> > long. I could use other's insights and thought others might enjoy the
>> > challenge. So here's the code:
>>
>> I think the problem is in the API. That's what makes it too complex.
>> There are too many cases of specific handling and options (see below):
>>
>> 1. Unnecessary option: if the key is supposed to stay intact the block
>> should just return the original key.
>
> I agree. Just wanted someone else to confirm. Out it goes!
:-)
>> > #
>> > # foo = { :name=>'Gavin', :wife=>:Lisa }
>> > # foo.rekey{ |k| k.to_s } #=> { "name"=>"Gavin", "wife"=>:Lisa }
>> > # foo #=> { :name =>"Gavin", :wife=>:Lisa }
>> > #
>> > # If no key map or block is given, then all keys are converted
>> > # to Symbols.
>>
>> 2. Why that default? In my mind this is too much implicit logic.
>> Also this can be easily achieved with
>>
>> hash.rekey(&:to_sym)
>
>
> I can understand that. I used the default b/c the vast majority of the time
> I was using it to convert to symbols. Rather then have yet another method
> like ActiveSupport's `symbolize_keys` it made more sense to me to just give
> #rekey this as the default.
>
> I can't really take that back now. It's been too long part of the API.
Well, then you mustn't change *anything* which affects observable behavior.
>> > #
>> > # Note that if both a +key_map+ and a block are given, the +key_map+
>> > is
>> > # applied first then the block.
>>
>> 3. I would change that to EITHER block OR map argument, but not both.
>
>
> Yea, I thought about that when I added support for the mapping. That was
> actually something that came later that the original block form. At first I
> had thought about making an entirely different method, but then thought that
> was a waste and that it made more sense as part of #rekey. So when I first
> added it I did an "either or", just as you suggest. But then I thought
> "why?" it certainly can handle both even if people will almost never use
> both.
>
> What do you think? Does it really matters enough to change it now? Like I
> said, I doubt anyone has used both, so this is something that could be
> change if it really is worth it.
I would get rid of that behavior. After all, if someone wants do do
Hash lookups as part of conversion they can still do that in the block
without much effort.
I also noticed a slight asymmetry between block and Hash: a block will
return something for every key passed in. But since a Hash is fixed
at the time of method call (modulo default_proc of course) there is
not really a way to handle absent keys ad hoc.
>> > Not the use of `facets/na`. That is defined as:
>> >
>> > class << NA = ArgumentError.new
>> > def inspect ; 'N/A' ; end
>> > def method_missing(*); self; end
>> > end
>> >
>> > But it is really nothing more than a dummy object used to mean Not
>> > Applicable. So in the case of #rekey, if the block returns NA then the
>> > key
>> > goes unchanged. Thinking about it again now, it's probably unnecessary,
>> > but
>> > I had wanted a way to say "leave it alone" while also making sure that
>> > `nil`
>> > could still be used as a key (even if that's rare). Feel free to remove
>> > the
>> > NA business, but if you do please explain why you think its not needed.
>>
>> If one needs a special key one can use a Symbol for that as
>> efficiently as nil. nil is the value return if something is absent
>> and I believe it does not make for a good key in a Hash.
>
>
> Wouldn't use symbols b/c then you have a special exception. NA was made just
> for such cases. But you probably right that `nil` doesn't make a good hash
> key no matter what. Nonetheless, it doesn't really matter b/c as you said
> above, they can just return the original key.
>
>>
>> > Best solution will get their name put in front of CREDITs for the next
>> > release of Facets.
>>
>> :-)
>>
>> class Hash
>> def rekey(mapping = nil, &convert)
>> c = convert || mapping
>> dup.tap do |h| # preserve type and defaults
>> h.clear
>> each_pair {|k, v| h[c[k] || k] = v}
>> end
>> end
>> end
>>
>
> Sweet. Much smaller than mine, that's for damn sure!!!
Yeah, but that's easy when one removes the complexity in behavior.
> Put the default
> :to_sym back in and we could have a deal :-)
Well, that's easy, isn't it?
def rekey(mapping = nil, &convert)
c = convert || mapping || :to_sym.to_proc
> I'm need to test and benchmark it first though.
Have fun!
> Oh, and nice use of polymorphism using #[] for both proc and hash retrieval!
Thank you! :-)
Kind regards
robert
--
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/
-- You received this message because you are subscribed to the Google Groups
ruby-talk-google group. To post to this group, send email to
[email protected]. To unsubscribe from this group, send email
to [email protected]. For more options, visit this
group at https://groups.google.com/d/forum/ruby-talk-google?hl=en
