Ruby counting duplicates in array and print into new file

Your for loop does not have a body so your last couple of lines reference i outside the loop where it is not defined.

The problem is with these lines.

registernums.print book[i][email protected]_i + ", "
registernums.print book[i][email protected]_i + ", "

To access any objects instance variables, you do not need to put an @. So the correct code should be

registernums.print book[i].ticketnum.to_i + ", "
registernums.print book[i].serialnumber.to_i + ", "

Also as @Jonah pointed out, there should be an end to end the last for loop.

Oh, boy!

Before I start few important points: - You are overusing instance variables - Ticket class is all right, but Ticketbook (should be TicketBook) should only have one instance_variable (the one set in initialize method), the rest should be local to method's scope.

• Ruby naming convention is to separate words with _ (new_doc, ticket_file and so on)

• You should almost never use for loop - the only reason to use it is to write your own iterator, but you are acting on arrays here - use each method

  • Use indentation!

Now about the errors:

ticketfile = File.open("tickets.txt", "r")

book = Ticketbook.new
ticketfile.each {|line| 
  a = line.split(",")

  newdoc = Ticket.new(a[0].to_i,a[1].to_i)
  book.newticket(newdoc)
}                                             

registernums = File.new("registernums.txt", "w")


for i in ([email protected])                       #  @ticketbook is an instance variable of Ticketbook, you'll get undefined length for nil:NilClass
registernums.print book[i][email protected]_i + ", "        # book is an instance of Ticketbook, [] is not defined on that class!
registernums.print book[i][email protected]_i + ", "
registernums.puts book[i].soldnumber(i)
print registernums

Your Ticketbook class

class Ticketbook

  def initialize
  @ticketbook = Array.new    #personaly would prefer []
  end

  def newticket(ticket)
  @ticketbook << ticket
  @ticketbook.sort! {|x,y| x.ticknum*1000 + x.serialnum <=> y.ticknum*1000 + y.serialnum}

  end

  def soldnumber(tickenum2,serialnum2)
        @ticknum2 = ticknum2           # unnecessary
        @serialnumb2 = serialnum2      # unnecessary
        @antal = 0
      for i in [email protected]      # Should be @ticketbook.each do |ticket|
      if @ticknum2 == @ticketbook[i].ticknum && @serialnum2 == @ticketbook[i].serialnum
          @antal +=1
      end
      end
      @antal

      # much better would be:
      # def soldnum(ticknum2, serialnum2)
      #   @ticketbook.select {|ticket| ticket.ticknum == ticknum2 && ticket.serialnum == serialnum }.count
      # end  

  end

end

I would also introduce you to group_by method - run on array will convert it into a really nice hash, where keys are result of executed block:

[1,2,3,4,5,6].group_by {|e| e.odd?} #=> {true => [1,3,5], false => [2,4,6]}

You can use it to get repetition count in one go:

# inside ticket book
def count_repetitions
  Hash[@ticketbook.group_by {|e| [e.ticknum, e.serialnum]}.map {|key, value| [key, value.count]}
end

This should return hash, where keys are two-element arrays containing ticknum and serialnum, and values are number of occurrences

There are a few problems here:

for i in ([email protected])
  registernums.print book[i][email protected]_i + ", "
  registernums.print book[i][email protected]_i + ", "
  registernums.puts book[i].soldnumber(i)
  print registernums

This code is outside the TicketBook class, so none of the instance variables (those beginning by @) are actually available.

If you want to access the array of tickets from outside the TicketBook, create an

attr_reader :ticketbook

in the TicketBook class.

You might want to replace your code by something like:

book.ticketbook.each_with_index do |tb, i|
  registernums.print tb.ticketnum.to_i + ", "
  registernums.print tb.ticketnum.to_i + ", "
  registernums.puts tb.soldnumber(i)
end

tIVAR refers to an instance variable, so the error message unexpected tIVAR means that ruby wasn't expecting an instance variable somewhere, and it points to this line (and the one after)

registernums.print book[i][email protected]_i + ", "

Accessing attributes in an object doesn't use the @ character (and it isn't part of the variable name either). A correct way to access your ticketnum attribute is

registernums.print book[i].ticketnum.to_i + ", "

As your question has been answered, I would like to suggest a more "Ruby-like" way of dealing with your problem. First, a few points:

• A ticket as an object having two attributes: a ticket number and a serial number. You could make a ticket an instance of a Ticket class, as you have done, a two-element array, with the understanding that the first and second elements correspond to the ticket and serial numbers, respectively, or as a hash, with one key for ticket number and another for serial number. I favor a hash. I see no need for separate class and I think the use of an array would be more likely to result in coding errors (e.g., if you used the wrong array element for a ticket or serial number).
• With all the methods Ruby provides in its Enumerable "mixin" module, there is no need to loop over indices. Avoiding such loops will make your code more compact, easier to read and less likely to contain errors.
• As someone else mentioned, you do not need (any) instance variables.

We begin by adding some tickets to ticketbook:

ticketbook = []
ticketbook << {tnbr: 22, snbr: 55}
ticketbook << {tnbr: 27, snbr: 65}
ticketbook << {tnbr: 22, snbr: 56}
ticketbook << {tnbr: 27, snbr: 66}
  # => [{:tnbr=>22, :snbr=>55}, {:tnbr=>27, :snbr=>65}, \
        {:tnbr=>22, :snbr=>55}, {:tnbr=>27, :snbr=>65}]

Now to find duplicates (tickets having the same ticket number but different serial numbers). Once you gain more experience with Ruby, you will think of the Enumerable#group_by method (or possibly Enumerable#chunk) whenever you want to group elements of an array by some characteristic:

g0 = ticketbook.group_by {|t| t[:tnbr]}
  # => {22=>[{:tnbr=>22, :snbr=>55}, {:tnbr=>22, :snbr=>56}], \
  #     27=>[{:tnbr=>27, :snbr=>65}, {:tnbr=>27, :snbr=>66}]} 

As you see, when we group_by ticket number, we obtain a hash with elements (k,v), where the key k is a ticket number and the value v is an array of tickets (hashes) having that ticket number.

This may be all you need. If you want a count of the numbers of tickets having the same serial number, you could use Enumerable#map to convert each value in the g0 hash (an array of tickets having the same ticket number) to the number of such tickets:

g1 = g0.map {|k,v| {k => v.size}} # => [{22=>2}, {27=>2}]

You might stop here, but it would be more convenient if this were instead a hash ({22=>2, 27=>2}), rather than an array of single-pair hashes. There are several ways you can convert this array to a hash. One is to use map to convert the hashes to arrays:

g2 = g1.map(&:to_a) # => [[[22, 2]], [[27, 2]]]

(where map(&:to_a) is shorthand for map {|h| h.to_a}) then use Array#flatten to convert this to:

g3 = g2.flatten # => [22, 2, 27, 2]

One way to create a hash (in general) is like this:

Hash[1,2,3,4] # => {1=>2, 3=>4}

To do this with the array g3 we need to prepend the array with the "splat" operator:

Hash[*g3] # => {22=>2, 27=>2}

This gives us the desired hash of counts by ticket number. I said that was one way to convert an array of single-pair hashes to a hash. Here is a more direct way:

g1.pop.merge(*g1) # => {27=>2, 22=>2}

Here g1.pop returns {27=>2} and converts g1 to [{22=>2}]. The above expression is therefore equivalent to:

{27=>2}.merge(*[{22=>2}]) # => {27=>2, 22=>2}

which merges the hashes in the splatted array (here just one) into the hash that precedes merge.

Rather than introducing the local variables g0 and g1, you would normally "chain" these three operations:

ticketbook.group_by {|t| t[:tnbr]}.map {|k,v| {k => v.size}}.pop.merge(*g1)
  # => {27=>2, 22=>2}  

Lastly, while your version of sort is fine, you could also do it like this:

ticketbook.sort! {|x,y| (x <=> y) == 0 ? x[:snbr] <=> y[:snbr] : \
  x[:tnbr] <=> y[:tnbr]}