HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Best match between listed companies and private ones based on 3 ratios

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
onesmatchprivateratioscompaniesbetweenlistedbasedandbest

Problem

My code finds the best match between a private company and a listed company based on 3 ratios in order to report the ratio4 from the listed to the private company.

In order to do that first, I do the percentile of the ratio1 of listed company (as it is the most important).

Secondly, I check in which percentile the value of the ratio1 of the private company is in.

Then, there are two options:

  • The perfect match, meaning that the sum of the difference of the 3 ratios equal to 0 then I report the ratio4 of this listed company to this private one.



  • Find the minimum of the sum of the difference of all ratios.



```
Option Explicit

Sub EquityVol_Supplier()

Dim resultCell As Double
Dim CheckCell As Double, CheckCell2 As Double, checkCell3 As Double
Dim bestDiff4 As Double
Dim i As Integer, j As Integer
Dim dLowValue As Double
Dim LastSupplier As Long, LastListed As Long
Dim ratio1 As Double, ratio2 As Double, ratio3 As Double
Dim Name As String
Dim ebitsales9 As Double, ebitsales8 As Double, ebitsales7 As Double, ebitsales6 As Double
Dim ebitsales5 As Double, ebitsales4 As Double, ebitsales3 As Double, ebitsales2 As Double, ebitsales1 As Double
Dim arraylist

'Find the last row of suplier name
LastSupplier = Range("H" & Rows.Count).End(xlUp).Row
'Find the last row of Company listed
LastListed = Range("D" & Rows.Count).End(xlUp).Row

On Error Resume Next
For j = 2 To LastSupplier

dLowValue = 100000

'Ratio 1, 2 and 3 of supplier
CheckCell = Range("H" & j).Value
CheckCell2 = Range("I" & j).Value
checkCell3 = Range("J" & j).Value

'Define the array of the ratio 1 (the same as the ratio1 of supplier) of Listed company
arraylist = Range("A2:" & "A" & LastSupplier)

'Define the percentile of ratio1 of listed companies
ebitsales9 = Application.WorksheetFunction.Percentile(arraylist, 0.9)
ebitsales8 = Application.WorksheetFunction.Percentile(arraylist, 0.8)
ebitsales7 = Application.WorksheetFunction.Percentile(arraylist, 0.

Solution

Firstly, this would be much easier to understand if you included an example/screenshot of the underlying data. But anyway,

Things I like

Option Explicit - always the first thing I check for. I honestly don't know why it was ever optional in the first place.

Frequent comment signposting - For really good code, the code should signpost itself, but for this, it makes your code very easy to follow and understand.

Good use of indenting and whitespace - Again, makes your code very readable.

Variable Naming

Your naming is pretty decent but could be better.

VBA Naming Conventions:

Procedure-level variables use Dim and are spelt in camelCase

Module-Level are Private and in PascalCase

Global-Level are Public and also PascalCase

Constants are Public Const and SHOUTY_SNAKE_CASE

Personally, I do Sheets, Books, Modules, Classes etc. like so ws_PascalCase but that's just personal preference. As long as you're consistent, it doesn't really matter how you do it.

So, referencing your code, resultCell, bestDiff4, dLowValue are good, CheckCell, ebitsales9, Name are not.

Meaningful Variable Names

Variable names should always be Clear, Consise and, above all, Unambiguous.

If I see resultCell, I'm going to assume it's, well, a Cell. I'm certainly not going to think it's a number. resultCellValue is suddenly very clear about what it is. Similarly LastSupplier, LastListed are not clear about what they are. lastSupplierRow, lastListedRow and suddenly I know exactly what they are.
ratio1, ratio2, ratio3. I at least know they're ratios, but ratios of what? Numbered variables are almost never appropriate. If you can name them, you should. I can't tell how your ratios are calculated, but they should be named something like ratioValueToValue.
Similarly, ebitsales1 etc. is both numbered and unclear. If I see that, I'm going to assume it's some kind of sales figure, not a percentile value. 10thPctlEbit would be much clearer. bestDiff4. Where's bestDiff 1 to 3?

Don't Repeat Yourself

A.K.A. DRY. Whenever you find yourself copy-pasting a block of code: Stop. Go to the bottom of your code module. Write a new Sub/Function to do what your code does.

For instance this:

'If bestdiff4=0 means perfect match so go next supplier
    If bestDiff4 = 0 Then
        dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Range("K" & j).Value = resultCell
        GoTo NextIteration

'if no perfect match so find the lowest sum of the difference of all ratios
    ElseIf bestDiff4 < dLowValue Then
       dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Name = Range("E" & i)
    End If


Can just be a subroutine like so:

Public Sub ProcessMinRatio(byval bestDiff4 as double, byval i as long, byval j as long)

'If bestdiff4=0 means perfect match so go next supplier
    If bestDiff4 = 0 Then
        dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Range("K" & j).Value = resultCell
        GoTo NextIteration

'if no perfect match so find the lowest sum of the difference of all ratios
    ElseIf bestDiff4 < dLowValue Then
       dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Name = Range("E" & i)
    End If

End Sub


then you can write

If CheckCell > ebitsales9 And Range("A" & i).Value > ebitsales9 Then
                bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
                ProcessMinRatio bestDiff4, i, j

    ElseIf CheckCell > ebitsales8 And Range("A" & i).Value > ebitsales8 Then
                bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
                ProcessMinRatio bestDiff4, i, j

    etc.


Then we can make it even cleaner:

If

ElseIf

ElseIf


Should be a Select... Case Statement like so:

Select Case CheckCell

Case Is > ebitsales9 
    bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
    ProcessMinRatio bestDiff4, i, j

Case is > ebitsales8
    bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
    ProcessMinRatio bestDiff4, i, j

etc.


You should also avoid using GoTo. How about using a boolean bestRatioIsFound and exiting your loop as soon as it's found?

bestRatioIsFound = false
i = 1
Do While i < lastListed and bestRatioIsFound = False
    i = i + 1

    ...

    If bestDiff4 = 0 Then
        dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Range("K" & j).Value = resultCell 

        bestRatioIsFound = true

    ...

Loop


Why are you finding the percentile at all? As best I can tell, you perform the exact same operations regardless of which percentile it's in, and you never store or output the percentile information anywhere.

Code Snippets

'If bestdiff4=0 means perfect match so go next supplier
    If bestDiff4 = 0 Then
        dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Range("K" & j).Value = resultCell
        GoTo NextIteration

'if no perfect match so find the lowest sum of the difference of all ratios
    ElseIf bestDiff4 < dLowValue Then
       dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Name = Range("E" & i)
    End If
Public Sub ProcessMinRatio(byval bestDiff4 as double, byval i as long, byval j as long)

'If bestdiff4=0 means perfect match so go next supplier
    If bestDiff4 = 0 Then
        dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Range("K" & j).Value = resultCell
        GoTo NextIteration

'if no perfect match so find the lowest sum of the difference of all ratios
    ElseIf bestDiff4 < dLowValue Then
       dLowValue = bestDiff4
        resultCell = Range("D" & i)
        Name = Range("E" & i)
    End If

End Sub
If CheckCell > ebitsales9 And Range("A" & i).Value > ebitsales9 Then
                bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
                ProcessMinRatio bestDiff4, i, j

    ElseIf CheckCell > ebitsales8 And Range("A" & i).Value > ebitsales8 Then
                bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
                ProcessMinRatio bestDiff4, i, j

    etc.
If

ElseIf

ElseIf
Select Case CheckCell

Case Is > ebitsales9 
    bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
    ProcessMinRatio bestDiff4, i, j

Case is > ebitsales8
    bestDiff4 = Application.WorksheetFunction.Min(ratio1 + ratio2 + ratio3)
    ProcessMinRatio bestDiff4, i, j

etc.

Context

StackExchange Code Review Q#112092, answer score: 3

Revisions (0)

No revisions yet.