Return the best factors of a whole number I've just finished writing my code to return the prime factors of a number and I feel the GetBestFactors function could be improved.
The GetFactors function was hacked together from the code found here:https://support.microsoft.com/en-us/kb/202782.  This returns all the factors of a number in a collection.

The GetBestFactors then loops through each number finding the best set to use.
So my question is - could the GetBestFactors function use less For...Next loops?

Sub Test()

    Dim tmp As Collection
    Set tmp = New Collection

    Dim tmp1 As Collection
    Set tmp1 = New Collection

    Set tmp = GetFactors(15) 'Change number - will return all factors.
    Set tmp1 = GetBestFactors(15) 'Change number - will return best factors.

    Debug.Assert False

End Sub

'Returns the factors of a whole number.
Public Function GetFactors(NumToFactor As Single) As Collection

    Dim Count As Integer
    Dim Factor As Single
    Dim y As Single
    Dim tmpCollection As Collection

    Set tmpCollection = New Collection

    Count = 0
    For y = 1 To NumToFactor
        Factor = NumToFactor Mod y
        If Factor = 0 Then
            tmpCollection.Add y
        End If
    Next y

    Set GetFactors = tmpCollection

End Function

'Returns the highest factors of a number.
Public Function GetBestFactors(NumToFactor As Single) As Collection

    Dim tmpFactors As Collection
    Dim FactorNums As Collection
    Dim x As Single, y As Single, z As Single
    Dim FirstFactor As Single

    Set tmpFactors = New Collection
    Set FactorNums = New Collection

    'Get all factors for the number.
    Set FactorNums = GetFactors(NumToFactor)

    'If the collection has 1 item then the NumToFactor is 1.
    'If there's 2 items then it's a prime number (1 and NumToFactor)
    If FactorNums.Count = 1 Or FactorNums.Count = 2 Then
        tmpFactors.Add FactorNums(FactorNums.Count)
    Else
        For x = FactorNums.Count - 1 To 1 Step -1
            If FactorNums(x) ^ 2 = NumToFactor Then
                tmpFactors.Add FactorNums(x)
                tmpFactors.Add FactorNums(x)
                Exit For
            Else
                For y = x To 1 Step -1
                    FirstFactor = FactorNums(y)
                    For z = y - 1 To 1 Step -1
                        If FirstFactor * FactorNums(z) = NumToFactor Then
                            tmpFactors.Add FirstFactor
                            tmpFactors.Add FactorNums(z)
                            Exit For
                        End If
                    Next z
                    If tmpFactors.Count = 2 Then Exit For
                Next y
            End If
            If tmpFactors.Count = 2 Then Exit For
        Next x
    End If

    Set GetBestFactors = tmpFactors

End Function
I don't understand the logic specifically

allFactors(15) = 1,3,5,15
bestFactors(15) = 3,5

allFactors(7) = 1,7
bestFactors(7) = 7

allFactors(1) = 1
bestFactors(1) = 1

allFactors(0) = empty
bestFactors(0) = empty

allFactors(-15) = empty
bestFactors(-15) = empty




All of your Single variables can be Long - single is hardly used in VBA.

Your naming could use some improvement:

Sub TestForFactors()

    Dim allFactors As Collection
    Set allFactors = New Collection

    Dim bestFactors As Collection
    Set bestFactors = New Collection

    Set allFactors = GetFactors(15) 'Change number - will return all factors.
    Set bestFactors = GetBestFactors(15) 'Change number - will return best factors.

    Debug.Assert False

End Sub


That's easier to understand.



Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names. Variable names - give your variables meaningful names. Instead of 

NumToFactor = numberToFactor
Count = count
Factor = factor
tmpCollection = getFactorsCollection
tmpFactors = bestFactorsCollection
FactorNums = factorNumbers
FirstFactor = firstFactor

x,y,z = ??




All of your procedures are Public - they should be Private unless needed to be Public.

All of your function arguments are being passed ByRef - they should be passed ByVal



In terms of your logic, you're looping through every number to find factors. There are certain rules you know you can abide by 
 - things that end with 0 or 5 
 - no primes end in an even number
 - If 5 is a factor, no need to test any multiples of 5 etc



You're using a lot of Collections - https://stackoverflow.com/questions/10579457/why-use-arrays-in-vba-when-there-are-collections you might want to try arrays or dictionaries.



In GetFactors what is Count doing? It's set to 0 then.. never used.



This is one heck of an arrow

    If FactorNums.Count = 1 Or FactorNums.Count = 2 Then
        tmpFactors.Add FactorNums(FactorNums.Count)
    Else
        For x = FactorNums.Count - 1 To 1 Step -1
            If FactorNums(x) ^ 2 = NumToFactor Then
                tmpFactors.Add FactorNums(x)
                tmpFactors.Add FactorNums(x)
                Exit For
            Else
                For y = x To 1 Step -1
                    FirstFactor = FactorNums(y)
                    For z = y - 1 To 1 Step -1
                        If FirstFactor * FactorNums(z) = NumToFactor Then
                            tmpFactors.Add FirstFactor
                            tmpFactors.Add FactorNums(z)
                            Exit For
                        End If
                    Next z
                    If tmpFactors.Count = 2 Then Exit For
                Next y
            End If
            If tmpFactors.Count = 2 Then Exit For
        Next x
    End If


And a lot of Exit For - there's four of those against 3 For Next loops - hard to follow which exits what loop.