tags:

views:

98

answers:

4

I have an application that I am using Active Record to access a database. I'm trying to take the information and put it into a hash for later use.

Here is basically what I am doing.

require 'active_support'
    @emailhash = Hash.new
    emails = Email.find(:all)
    emails.each do |email|
            email.attributes.keys.each do |@column|
                    @emailhash[email.ticketno] || Hash.new
                    @emailhash[email.ticketno] = email.@column
            end
    end

The line that doesn't work is:

@emailhash[email.ticketno] = email.@column

Is there any way that I can do this properly? Basically my goal is to build a hash off of the values that are stored in the table columns, any input is appreciated.

+2  A: 

You cannot have an iterator variable starting with an @. Try something like this:

require 'active_support'
@emailhash = Hash.new
emails = Email.find(:all)
emails.each do |email|
        @emailhash[email.ticketno] = email.attributes.keys.collect{|key| key.column}
end
blinry
in ruby 1.8 you can have an iterator variable starting with an @:(0..20).each { |@y| puts @y }
banister
+1  A: 

In addition to blinry's comment, the line

@emailhash[email.ticketno] || Hash.new

looks suspicious. Are you sure you don't want

@emailhash[email.ticketno] ||= Hash.new

?

liwp
+3  A: 
  • Ruby programmers usually indent 2
  • Your code was squishing all of the emails into one hash entry, rather than an entry per email.
  • If you want to call a method dynamically, use Object.send.
  • @emailhash[email.ticketno] || Hash.new doesn't do anything.

Something like this might do it:

require 'active_support'
@emailhash = {}
Email.find(:all).each do |email|
  @mailhash[email.ticketno] = {}
  email.attributes.keys.each do |key|
    @emailhash[email.ticketno][key] = email.send(key)
  end
end

The key piece is "send", which calls the method identified by a string or symbol.

Wayne Conrad
Thanks, this worked perfectly. Sorry about the typo also, I found that right after I started this thread. I will definitely remember the send function for the future.
Eugene
A: 

Besides the previous accurate observations, I would like to add the following:

Point 1.
Is important to state that @ivars may not work on formal function parameters... This said, I think it is invalid to have:

collection.each { |@not_valid| }

Is also a bad practice to have @ivars inside blocks, you won't know for sure in which context this block will be executed in (As you many know, the self reference inside that block may be different than the self reference outside it).

Point 2.
Another point that you should have in mind is that if you don't assign the result of a (||) operator this won't do any modification at all (just will be a time waster), however you could use:

mail_hash[email.ticketno] = mail_hash[email.ticketno] || Hash.new

That can be easily rewritten to:

mail_hash[email.ticketno] ||= Hash.new

Point 3.
Even if email.attributes.keys is a cheap instruction, is not free... I would suggest to have that outside the iteration block (given that the keys will always be the same for each record, given we are not using Document Databases).

Final Result

require 'active_support'

mails = Email.all
@mailshash = mails.inject(Hash.new) do |hsh, mail|
  # mail.attributes is already a representation of the 
  # email record in a hash
  hsh[mail.ticketno] = mail.attributes
  hsh
end
Roman Gonzalez
"Is also a bad practice to have @ivars inside blocks, you won't know for sure in which context this block will be executed in..." @Roman, If you're calling the standard library, there's no need to be paranoid. Array and Enumerable methods that take blocks don't do any monkey business. I don't know of any code in the standard library that does. Maybe some DSLs do.
Wayne Conrad
Granted, maybe is a bit paranoid, but I don't know, it just feels wrong for me to do that, even in standard library methods.
Roman Gonzalez