Two identical code giving completely different result

[JohnSeito]

Hi guys,

Sorry to bother you, I have been looking at this line of code for a few hours now and
I can't seem to get why one code is giving a correct solution and the other one
not even close. They are both identical but there are some difference.

Just wondering if anyone could see why that is?

The program is suppose to pick up about 11 cell values, it then store these
cell values into a collection, and randomly picking out the 11 values without repeating
until there is no more to pick out.

Here is the code: (the top one doesn't give the correct result, it picks it out of the same ones and
the number doesn't seem to store correctly, the bottom codes works as it suppose to work.)

lastCol = Cells(215, Columns.Count).End(xlToLeft).Column

Dim C As New Collection, I As Integer, rdom As Integer

rowFOUR = 216
colFOUR = 2
'cntHIT = 0
rowtest = 216
    
For I = 2 To lastCol - 1
    If Cells(rowFOUR, colFOUR) <> "" Then
        Cells(rowtest, "W") = colFOUR
            num = Cells(rowtest, "W")
        C.Add CStr(num), CStr(num)
        rowtest = rowtest + 1
    End If
    
    colFOUR = colFOUR + 1
Next I

n = 0
Row = 216
Do

    I = Int(Rnd * C.Count + 1)
    Cells(Row, "X") = I
    C.Remove (I)
    
    Row = Row + 1
    n = n + 1
Loop While n <= 15 'numTimes >= 0

Dim C As New Collection, I As Integer

Row = 215
For I = 1 To 11
    num = Cells(Row, "W")
  C.Add CStr(num), CStr(num)
  Row = Row + 1
Next I

I = Int(Rnd * C.Count + 1)
x = 216

Do
    Cells(x, "x") = C.Item(I)
    C.Remove (I)
    I = Int(Rnd * C.Count + 1)
    x = x + 1
Loop Until x >= 250

 

[s hal]

I can't grasp where the error might be. Have you tried stepping through your code one line at a time to isolate what its doing that you didn't expect? That's how I catch my bugs typically...

 

[JohnSeito]

yup, I run it numerous times line by line. The code has no errors, just when it start store the values into the collection, and then splitting/ outputting
it into excel, the values are not correct. It repeated some values and some values are suppose to be there, is there.

The bottom codes seems to be correct, the value it stores, is the value that comes out without repeating.

 

[xenou]

A collection allows duplicates. If you have selected duplicates in the collection, then you will output duplicates.
ξ

 

[JohnSeito]

yes you are correct, but after I output it, I removed it from the collection.

This is assumed in the beginning we only read in unique numbers. So suppose to be no duplicated outputs.
But for the second code it is, and is reading the same value as the code from the bottom.

 

[xenou]

This is assumed in the beginning we only read in unique numbers.

I don't see where you are guaranteeing unique numbers. It's hard to see what your intention here is. Is this a real world application? What are you doing with this?

 

[JohnSeito]

Sorry I worded it incorrectly. What I meant to say is that I've tested with real data, and all the datas are unique. But I think I should figure it out
with looking at it a little bit longer. I think the code might not have picked it out from the number I thought it was suppose to, that might be the problem.

Sorry for that !!

 

[JohnSeito]

I just tried check again, and I don't know what is wrong.

And sorry if I wasn't clear earlier, but these are the numbers I was reading into the collection.

2
8
9
11
12
13
14
15
16
18
19

and these number are given by traversing through a row of columns, and if the row doesn't have blanks, to come up with the row number.
So that is how those number came to be, I then output those number into a cell, and going downward.

After that I pick up those number cell by cell into a collection and then I output those number from the collection into the cell next to it row by row.
and the result I have is incorrect and it looks like this.

2 8
8 6
9 6
11 3
12 3
13 5
14 1
15 4
16 3
18 2
19 1
1

If Cells(rowFOUR, colFOUR) <> "" Then         ---------> this is the code that picks up the column that has no blanks, 
Cells(rowtest, "W") = colFOUR     --------------------->this is the code that puts it into column W, going downward, right here I could have store colFOUR into the collection but I experiment 
             ------------------------------------------------> but doesn't work so I did this extra step.
num = Cells(rowtest, "W")         -------------------> this is the code that picks it up from the cell that the value was put onto in the code above
C.Add CStr(num), CStr(num)       -------------------> this is the code that stores it into the collection


I = Int(Rnd * C.Count + 1)  ---------------------> This is randomly selecting from the collection
Cells(Row, "X") = I            -----------------------> this is when it puts it right back  in column X
C.Remove (I)               -----------------------> this is the code that removes the number that was put into column X from the collection

the output it split out is this
8
6
6
3
3
5
1
4
3
2
1
1


when is reading in on these lines of values,
2
8
9
11
12
13
14
15
16
18
19

I don't know why.

this code on the bottom works fine, this code is different in that it doesn't have a loop to find the non-blank columns.
it just picks the values from the cells, stores them into the collection, then output it.

Sub workingCODE()
    
Dim C As New Collection, I As Integer

Row = 215
For I = 1 To 11
    num = Cells(Row, "W")
  C.Add CStr(num), CStr(num)
  Row = Row + 1
Next I


I = Int(Rnd * C.Count + 1)
'MsgBox C.Item(I)
x = 216

Do
    Cells(x, "x") = C.Item(I)
    C.Remove (I)
    I = Int(Rnd * C.Count + 1)
    x = x + 1
Loop Until x >= 250


'MsgBox C.Item(I)


End Sub

Not sure why one gives the correct result, and the other one isn't.

 

[JohnSeito]

I have isolated the code and see which section of the code is causing the problem.
The code that is giving a wrong error is the problem code's bottom section , the
part that output's information.


I replace this section, the bottom of the bad code.

n = 0
Row = 216
Do

    I = Int(Rnd * C.Count + 1)
    Cells(Row, "X") = I
    C.Remove (I)
    
    Row = Row + 1
    n = n + 1
Loop While n <= 15 'numTimes >= 0

with this one, the good one of the bottom code and it works correctly.
So that means the top code is wrong (the bottom of the bad code), but I don't see why, is the same thing. :-/

I = Int(Rnd * C.Count + 1)
x = 216

Do
    Cells(x, "x") = C.Item(I)
    C.Remove (I)
    I = Int(Rnd * C.Count + 1)
    x = x + 1
Loop Until x >= 250

 

[Cool blue]

First thing I would say is that your indexing is not very clear. Its much better to use For Each structures for Collections and for the Output Range. This is then much easier to understand and you don't have to worry about matching the counting systems of the different Indices. Given the complicated indexing that you have in your two methods, its lucky that either of them works the way you want.

Second thing is that in your first piece of code, you output the randomly generated index and in the second you output the member of the collection at the random location and its a big assumption that these are the same thing.