4
\$\begingroup\$

I am trying to create a randomly generated array of x numbers with no duplicates. I am choosing numbers, one at a time, then comparing each new number to the numbers already in the array.

Here is my code. At the end I should have an array of seven numbers, none that match, but sometimes I get a nil:

array = [] #DEFINES THE ARRAY

array[0] = rand(10)  #THIS CHOOSES FIRST NUMBER (FN) AND PUTS IT IN THE FIRST ARRAY SPOT
p "fn = #{array[0]}"

for i in 1..6  #START OF FOR LOOP TO GET 6 NUMBERS IN THE ARRAY - WHY 6?  THE FIRST NUMBER THAT POPPED IN MY HEAD

  p "Iteration #{i} -------------------------"  # THIS IS JUST SO I KNOW WHERE I AM IN THE LOOPS

  @x = rand(10)  #THIS CHOOSES THE NEXT NUMBER AND ALL NUMBERS AFTER

  array.each do |uc|  # THIS IS THE LOOP THAT COMPARES ALL NUMBERS

    @type = @x == uc ? "yes" : "no"  #DOES THE CHOSEN NUMBER EQUAL ANY NUMBER IN THE ARRAY

    p "does #{uc} = #{@x}? #{@type}"
    if @type == "yes"  # IF THE COMPARE IS TRUE, I DON'T WANT ANYTHING DONE.  IT WILL CYCLE THRU AND GET A NEW NUMBER

      i = 1 
      p "YES #{@x} = #{uc}"
      break
    end  #END OF IF YES
  end  #END OF ARRAY EACH

  if @type == "no"  #IF NO, PUT NEXT NUMBER (@X) INTO THE NEXT ARRAY SPOT.

    p "in last if type= #{@type}"  #THESE STATEMENTS JUST PRINT OUT THE DIFFERENT VARIABLES SO I KNOW I AM GETTING WHAT I EXPECT

    p "in last if x = #{@x}"
    p "in last if i = #{i}"
    @x = array[i]  #THIS "SHOULD" FILL THE NEXT ARRAY SPOT - BUT DOESN'T SEEM TO

    p "#{array[i]} is in the array"  #THIS PRINT OUT IS BLANK - STATEMENT ABOVE DID NOT WORK.

    p array[i]
  end  #END OF IF NO

end  #END OF FOR LOOP

p array #PRINTS OUT THE CONTENTS OF THE ARRAY

I know there are probably quicker and easier ways to do this, but I am starting out with what I know, and building up.

\$\endgroup\$

3 Answers 3

6
\$\begingroup\$

I don't understand what you really want, but this simple one-liner generates an array of ten elements (from 1 to 10) with a random order. Tweak it to meet your needs:

(1..10).to_a.shuffle
#=> [4, 10, 1, 7, 3, 5, 8, 2, 9, 6]
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Actually it had to be (0...10).to_a.shuffle.take(7). \$\endgroup\$
    – Nakilon
    Commented Jan 15, 2013 at 18:54
  • \$\begingroup\$ Why did u append @steenslag answer? \$\endgroup\$
    – Nakilon
    Commented May 22, 2013 at 14:10
  • \$\begingroup\$ @Nakilon, I hadn't understood that the OP wanted only 7 elements (I am not sure yet, he didn't give feedback), your comment make my realize this. So I just completed it. \$\endgroup\$
    – tokland
    Commented May 22, 2013 at 17:22
  • \$\begingroup\$ @tokland, but everyone can realize the best solution – why to accumulate answers, that were already posted by another people? \$\endgroup\$
    – Nakilon
    Commented May 22, 2013 at 17:52
9
\$\begingroup\$

The sample method does what you want:

(1..10).to_a.sample(7) #=> [2, 9, 1, 6, 8, 10, 4]
\$\endgroup\$
3
  • \$\begingroup\$ That could return duplicates I believe. \$\endgroup\$
    – kamoroso94
    Commented Feb 22, 2018 at 14:32
  • 2
    \$\begingroup\$ @kamoroso94 from the docs: "(...) in order to ensure that an element doesn’t repeat itself unless the array already contained duplicate elements." \$\endgroup\$
    – steenslag
    Commented Feb 22, 2018 at 17:29
  • \$\begingroup\$ thanks for the correction! I'm still learning Ruby, so I must have misremembered. \$\endgroup\$
    – kamoroso94
    Commented Feb 22, 2018 at 17:32
4
\$\begingroup\$
  1. Why do you put empty lines everywhere? They don't make your code more readable.
  2. If you used @ not because you are writing classes, but just to fix scope problems, you may try either use $ instead or even better define type variable outside the nested loop (before array.each do |uc|).
  3. And the main thing. Ruby's loops are more highlevel, than in, for example, C. And you can't directly change the i value. So you should use keyword redo to repeat the iteration. http://www.ruby-doc.org/docs/ProgrammingRuby/html/tut_containers.html
  4. You may use puts instead of p to get rid of "", but it may need STDOUT.sync = true fix under Windows.
  5. Also you need array[i] = x instead of x = array[i]. If you aren't experienced in programming, I would recommend to start with C/C++.

So here is your program. Not refactored yet, but fixed.

array = []
array[0] = rand 10
puts "fn = #{array[0]}"
for i in 1..6
  puts "Iteration #{i} -------------------------"
  x = rand 10
  array.each do |uc|
    $type = x == uc ? "yes" : "no"
    puts "does #{uc} = #{x}? #{$type}"
    if $type == "yes"
      puts "YES #{x} = #{uc}"
      break
    end
  end
  if $type == "no"
    puts "in last if type= #{$type}"
    puts "in last if x = #{x}"
    puts "in last if i = #{i}"
    array[i] = x
    puts "#{array[i]} is in the array"
    p array[i]
  else
    redo
  end
end
p array

Now refactoring:

You need to know, that Ruby arrays have a method #include?, which you may use instead of loop. We will lose debugging ability a bit, but it makes program more pretty, and now we don't need to make type variable global.

array = []
array[0] = rand 10
puts "fn = #{array[0]}"
for i in 1..6
  puts "Iteration #{i} -------------------------"
  x = rand 10
  type = array.include?(x) ? "yes" : "no"
  puts "does array include #{x}? #{type}"
  if type == "no"
    puts "in last if type=#{type}, x=#{x}, i=#{i}"
    array[i] = x
    puts "#{array[i]} is in the array"
  else
    redo
  end
end
p array

Then get rid of "yes" and "no", and use native Ruby true and false. And you may add tap to any object for debugging purposes - this universal method returns the object itself after execution, so you will be able to comment it later via putting # before .tap (when you don't need debug output) without breaking the program.

array = []
array[0] = rand 10
puts "fn = #{array[0]}"
for i in 1..6
  puts "Iteration #{i} -------------------------"
  x = rand 10
  if (type = array.include? x).tap{ puts "does array include #{x}? #{type : "yes" : "no"}" }
    redo
  else
    puts "in last if type=#{type}, x=#{x}, i=#{i}"
    array[i] = x
    puts "#{array[i]} is in the array"
  end
end
p array

But actually you print the type the second time only when it is false, so don't do it. Also we don't need else in this case, because redo will throw us away from this iteration. And actually we can start from iteration 0 and then change for to times. But we will lose the puts "fn=" (I don't know, whether you really need it).

array = []
7.times do |i|
  puts "Iteration #{i} -------------------------"
  x = rand 10
  redo if array.include?(x).tap{ |type| puts "does array include #{x}? #{type ? "yes" : "no"}" }
  puts "x=#{x}, i=#{i}"
  array[i] = x
  puts "#{array[i]} is in the array"
end
p array
\$\endgroup\$
1
  • \$\begingroup\$ Outstanding. Thank you very much for showing me how my code was wrong in the first place, and then going farther by simplifying it in easy to understand steps. I learned much, and I greatly appreciate it. \$\endgroup\$
    – Kyzer
    Commented Jan 15, 2013 at 17:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.