0
\$\begingroup\$

Long piece of code, I hope it is readable.

The code is basically if statements looking at 4 arrays: ArrPnLDataD1, PricingDatesArr, ArrForwardCurves and ArrHistoricalPrices.

I thought that having gone with arrays, the code would run quite fast but due to the size of the arrays and the number of loops it is running for too long. Any suggestions on how to optimize?

For k = LBound(ArrPnLDataD1, 1) To UBound(ArrPnLDataD1, 1)

    For j = LBound(PricingDatesArr, 1) To UBound(PricingDatesArr, 1)
    
    If ArrPnLDataD1(k, 158) <> "N/A Bio Element" Then
    
        If ArrPnLDataD1(k, 43) = "Phys - Time Based - Fixed Forward Price" Or ArrPnLDataD1(k, 43) = "Phys - Movement Based - Fixed Forward Price" Then 'If FWD Fixed price case
        
            For n = 2 To UBound(ArrForwardCurves, 2)
        
            If ArrPnLDataD1(k, 77) = ArrForwardCurves(1, n) And PricingDatesArr(j, 4) = ArrForwardCurves(3, n) Then 'Curve 1
        
                For x = 8 To UBound(ArrForwardCurves, 1)
        
                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                
                PricingDatesArr(j, 10) = ArrForwardCurves(x, n) 'Populate curve 1 Price
                
                End If
                
                Next x
                
            ElseIf ArrPnLDataD1(k, 83) = ArrForwardCurves(1, n) And PricingDatesArr(j, 4) = ArrForwardCurves(3, n) Then 'Curve 2

                For x = 8 To UBound(ArrForwardCurves, 1)
        
                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                
                PricingDatesArr(j, 11) = ArrForwardCurves(x, n) 'Populate curve 2 Price
                
                End If
                
                Next x
    
            ElseIf ArrPnLDataD1(k, 89) = ArrForwardCurves(1, n) And PricingDatesArr(j, 4) = ArrForwardCurves(3, n) Then 'Curve 3

                For x = 8 To UBound(ArrForwardCurves, 1)
        
                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                
                PricingDatesArr(j, 12) = ArrForwardCurves(x, n) 'Populate curve 3 Price
                
                End If
                
                Next x
                
            ElseIf ArrPnLDataD1(k, 95) = ArrForwardCurves(1, n) And PricingDatesArr(j, 4) = ArrForwardCurves(3, n) Then 'Curve 4

                For x = 8 To UBound(ArrForwardCurves, 1)
        
                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                
                PricingDatesArr(j, 13) = ArrForwardCurves(x, n) 'Populate curve 4 Price
                
                End If
                
                Next x
                
            ElseIf ArrPnLDataD1(k, 101) = ArrForwardCurves(1, n) And PricingDatesArr(j, 4) = ArrForwardCurves(3, n) Then 'Curve 5

                For x = 8 To UBound(ArrForwardCurves, 1)
        
                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                
                PricingDatesArr(j, 14) = ArrForwardCurves(x, n) 'Populate curve 5 Price
                
                End If
                
                Next x
                
            End If
            
            Next n
            
        Else ' not forward fixed price

        If PricingDatesArr(j, 2) > ReportDate Then 'Pricing dates is in the future

            For n = 2 To UBound(ArrForwardCurves, 2)

            If ArrPnLDataD1(k, 77) = ArrForwardCurves(1, n) And PricingDatesArr(j, 3) = ArrForwardCurves(3, n) Then 'Curve 1

                For x = 8 To UBound(ArrForwardCurves, 1)

                If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then

                PricingDatesArr(j, 10) = ArrForwardCurves(x, n) 'Populate curve 1 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 83) = ArrForwardCurves(1, n) And PricingDatesArr(j, 3) = ArrForwardCurves(3, n) Then 'Curve 2

                For x = 8 To UBound(ArrForwardCurves, 1)

                If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then

                PricingDatesArr(j, 11) = ArrForwardCurves(x, n) 'Populate curve 2 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 89) = ArrForwardCurves(1, n) And PricingDatesArr(j, 3) = ArrForwardCurves(3, n) Then 'Curve 3

                For x = 8 To UBound(ArrForwardCurves, 1)

                If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then

                PricingDatesArr(j, 12) = ArrForwardCurves(x, n) 'Populate curve 3 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 95) = ArrForwardCurves(1, n) And PricingDatesArr(j, 3) = ArrForwardCurves(3, n) Then 'Curve 4

                For x = 8 To UBound(ArrForwardCurves, 1)

                If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then

                PricingDatesArr(j, 13) = ArrForwardCurves(x, n) 'Populate curve 4 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 101) = ArrForwardCurves(1, n) And PricingDatesArr(j, 3) = ArrForwardCurves(3, n) Then 'Curve 5

                For x = 8 To UBound(ArrForwardCurves, 1)

                If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then

                PricingDatesArr(j, 14) = ArrForwardCurves(x, n) 'Populate curve 5 Price

                End If

                Next x

            End If

            Next n


        ElseIf PricingDatesArr(j, 2) <= ReportDate Then  'Pricing dates is in the past

            For n = 2 To UBound(ArrHistoricalPrices, 2)

            If ArrPnLDataD1(k, 77) = ArrHistoricalPrices(1, n) Then 'Curve 1

                For x = 6 To UBound(ArrHistoricalPrices, 1)

                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then

                PricingDatesArr(j, 10) = ArrForwardCurves(x, n) 'Populate curve 1 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 83) = ArrHistoricalPrices(1, n) Then 'Curve 2

                For x = 6 To UBound(ArrHistoricalPrices, 1)

                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then

                PricingDatesArr(j, 11) = ArrForwardCurves(x, n) 'Populate curve 2 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 89) = ArrHistoricalPrices(1, n) Then 'Curve 3

                For x = 6 To UBound(ArrHistoricalPrices, 1)

                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then

                PricingDatesArr(j, 12) = ArrForwardCurves(x, n) 'Populate curve 3 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 95) = ArrHistoricalPrices(1, n) Then 'Curve 4

                For x = 6 To UBound(ArrHistoricalPrices, 1)

                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then

                PricingDatesArr(j, 13) = ArrForwardCurves(x, n) 'Populate curve 4 Price

                End If

                Next x

            ElseIf ArrPnLDataD1(k, 101) = ArrHistoricalPrices(1, n) Then 'Curve 5

                For x = 6 To UBound(ArrHistoricalPrices, 1)

                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then

                PricingDatesArr(j, 14) = ArrForwardCurves(x, n) 'Populate curve 5 Price

                End If

                Next x

            End If

            Next n
            
        End If
    
    End If
    
    End If
    
    Next j
    
Next k
    
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Hi, as @Freeflow points out, it's hard to say how to optimise this without knowing what the data looks like. Would you be able to explain the structure of those 4 key arrays - you don't have to give real data but describe header rows, dimensions (size), datatype, meaning etc. Presumably you do some reading and writing to/from a worksheet, in CodeReview the context (who calls your code and how) is as important as the code itself sometimes. There are a lot of magic numbers ArrPnLDataD1(k, 77), For x = 8 To ..., ArrPnLDataD1(k, 158) <> "N/A Bio Element" I have no idea what 77, 88,158 mean \$\endgroup\$ Commented Mar 16, 2022 at 11:57
  • \$\begingroup\$ Nested looping is always going to perform badly. You would need a totally different approach. If you are indeed using Excel then you should leverage formulas where possible. You can even use UDFs in cells to leverage VBA in your formulas without looping through arrays. \$\endgroup\$ Commented Mar 16, 2022 at 15:57
  • 1
    \$\begingroup\$ There is too much missing from this code to be able to advise you on how to optimize the code. Without knowing more about the data and the program structure we really can't advise you. \$\endgroup\$ Commented Apr 30, 2022 at 14:48
  • 2
    \$\begingroup\$ (hope[the code]is readable Why is it double spaced?) \$\endgroup\$ Commented May 1, 2022 at 20:12

3 Answers 3

0
\$\begingroup\$

Unsure how many rows are in PricingDatesArr, but I would start by moving If ArrPnLDataD1(k, 158) <> "N/A Bio Element" Then outside the For j loop. This will prevent iterating through all rows of the PricingDatesArr without doing any work.

And PricingDatesArr(j, 4) = ArrForwardCurves(3, n) and And PricingDatesArr(j, 3) = ArrForwardCurves(3, n) are listed in each of their respective If Then statements, so that can be moved to it's own outer If Then statement (although this creates another layer of nested If statements).

For k = LBound(ArrPnLDataD1, 1) To UBound(ArrPnLDataD1, 1)
    If ArrPnLDataD1(k, 158) <> "N/A Bio Element" Then
        For j = LBound(PricingDatesArr, 1) To UBound(PricingDatesArr, 1)
            If ArrPnLDataD1(k, 43) = "Phys - Time Based - Fixed Forward Price" Or ArrPnLDataD1(k, 43) = "Phys - Movement Based - Fixed Forward Price" Then 'If FWD Fixed price case
                For n = 2 To UBound(ArrForwardCurves, 2)
                    If PricingDatesArr(j, 4) = ArrForwardCurves(3, n) Then
                        If ArrPnLDataD1(k, 77) = ArrForwardCurves(1, n) Then 'Curve 1
                            For x = 8 To UBound(ArrForwardCurves, 1)
                                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                                    PricingDatesArr(j, 10) = ArrForwardCurves(x, n) 'Populate curve 1 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 83) = ArrForwardCurves(1, n) Then 'Curve 2
                            For x = 8 To UBound(ArrForwardCurves, 1)
                                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                                    PricingDatesArr(j, 11) = ArrForwardCurves(x, n) 'Populate curve 2 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 89) = ArrForwardCurves(1, n) Then 'Curve 3
                            For x = 8 To UBound(ArrForwardCurves, 1)
                                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                                    PricingDatesArr(j, 12) = ArrForwardCurves(x, n) 'Populate curve 3 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 95) = ArrForwardCurves(1, n) Then 'Curve 4
                            For x = 8 To UBound(ArrForwardCurves, 1)
                                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                                    PricingDatesArr(j, 13) = ArrForwardCurves(x, n) 'Populate curve 4 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 101) = ArrForwardCurves(1, n) Then 'Curve 5
                            For x = 8 To UBound(ArrForwardCurves, 1)
                                If ArrPnLDataD1(k, 158) = ArrForwardCurves(x, 1) Then
                                    PricingDatesArr(j, 14) = ArrForwardCurves(x, n) 'Populate curve 5 Price
                                End If
                            Next x
                        End If
                    End If
                Next n
            Else ' not forward fixed price
                If PricingDatesArr(j, 2) > ReportDate Then 'Pricing dates is in the future
                    For n = 2 To UBound(ArrForwardCurves, 2)
                        If PricingDatesArr(j, 3) = ArrForwardCurves(3, n) Then
                            If ArrPnLDataD1(k, 77) = ArrForwardCurves(1, n) Then 'Curve 1
                                For x = 8 To UBound(ArrForwardCurves, 1)
                                    If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then
                                        PricingDatesArr(j, 10) = ArrForwardCurves(x, n) 'Populate curve 1 Price
                                    End If
                                Next x
                            ElseIf ArrPnLDataD1(k, 83) = ArrForwardCurves(1, n) Then 'Curve 2
                                For x = 8 To UBound(ArrForwardCurves, 1)
                                    If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then
                                        PricingDatesArr(j, 11) = ArrForwardCurves(x, n) 'Populate curve 2 Price
                                    End If
                                Next x
                            ElseIf ArrPnLDataD1(k, 89) = ArrForwardCurves(1, n) Then 'Curve 3
                                For x = 8 To UBound(ArrForwardCurves, 1)
                                    If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then
                                        PricingDatesArr(j, 12) = ArrForwardCurves(x, n) 'Populate curve 3 Price
                                    End If
                                Next x
                            ElseIf ArrPnLDataD1(k, 95) = ArrForwardCurves(1, n) Then 'Curve 4
                                For x = 8 To UBound(ArrForwardCurves, 1)
                                    If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then
                                        PricingDatesArr(j, 13) = ArrForwardCurves(x, n) 'Populate curve 4 Price
                                    End If
                                Next x
                            ElseIf ArrPnLDataD1(k, 101) = ArrForwardCurves(1, n) Then 'Curve 5
                                For x = 8 To UBound(ArrForwardCurves, 1)
                                    If PricingDatesArr(j, 2) = ArrForwardCurves(x, 1) Then
                                        PricingDatesArr(j, 14) = ArrForwardCurves(x, n) 'Populate curve 5 Price
                                    End If
                                Next x
                            End If
                        End If
                    Next n
                ElseIf PricingDatesArr(j, 2) <= ReportDate Then  'Pricing dates is in the past
                    For n = 2 To UBound(ArrHistoricalPrices, 2)
                        If ArrPnLDataD1(k, 77) = ArrHistoricalPrices(1, n) Then 'Curve 1
                            For x = 6 To UBound(ArrHistoricalPrices, 1)
                                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then
                                    PricingDatesArr(j, 10) = ArrForwardCurves(x, n) 'Populate curve 1 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 83) = ArrHistoricalPrices(1, n) Then 'Curve 2
                            For x = 6 To UBound(ArrHistoricalPrices, 1)
                                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then
                                    PricingDatesArr(j, 11) = ArrForwardCurves(x, n) 'Populate curve 2 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 89) = ArrHistoricalPrices(1, n) Then 'Curve 3
                            For x = 6 To UBound(ArrHistoricalPrices, 1)
                                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then
                                    PricingDatesArr(j, 12) = ArrForwardCurves(x, n) 'Populate curve 3 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 95) = ArrHistoricalPrices(1, n) Then 'Curve 4
                            For x = 6 To UBound(ArrHistoricalPrices, 1)
                                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then
                                    PricingDatesArr(j, 13) = ArrForwardCurves(x, n) 'Populate curve 4 Price
                                End If
                            Next x
                        ElseIf ArrPnLDataD1(k, 101) = ArrHistoricalPrices(1, n) Then 'Curve 5
                            For x = 6 To UBound(ArrHistoricalPrices, 1)
                                If PricingDatesArr(j, 2) = ArrHistoricalPrices(x, 1) Then
                                    PricingDatesArr(j, 14) = ArrForwardCurves(x, n) 'Populate curve 5 Price
                                End If
                            Next x
                        End If
                    Next n
                End If
            End If
        Next j
    End If
Next k

```
\$\endgroup\$
1
  • 1
    \$\begingroup\$ You have taken a good shot at it, but the question itself should be closed because there isn't enough information to provide a good answer. Not all questions on Code Review should be answered. \$\endgroup\$ Commented Apr 30, 2022 at 14:59
0
\$\begingroup\$

You don't provide details on the size of the arrays or the time scale.

As a first suggestion I'd move the above code to a DLL written in twinbasic or c#.

\$\endgroup\$
0
\$\begingroup\$

Shouldn't that be about the same as

    For k = LBound(ArrPnLDataD1, 1) To UBound(ArrPnLDataD1, 1)
        MeaningfulName158 = ArrPnLDataD1(k, 158)
        If MeaningfulName158 = "N/A Bio Element" Then
            next k
        End If
        MeaningfulName77 = ArrPnLDataD1(k, 77)
        MeaningfulName83 = ArrPnLDataD1(k, 83)
        MeaningfulName89 = ArrPnLDataD1(k, 89)
        MeaningfulName95 = ArrPnLDataD1(k, 95)
        MeaningfulName101= ArrPnLDataD1(k,101)
        For j = LBound(PricingDatesArr, 1) To UBound(PricingDatesArr, 1)
            If ArrPnLDataD1(k, 43) = "Phys - Time Based - Fixed Forward Price"
            Or ArrPnLDataD1(k, 43) = "Phys - Movement Based - Fixed Forward Price" 
            Then 'FWD Fixed price case
                For n = 2 To UBound(ArrForwardCurves, 2)
                    If PricingDatesArr(j, 4) <> ArrForwardCurves(3, n) Then
                        next n
                    End If
                    c = -1
                    MeaningfulName1 = ArrForwardCurves(1, n)
                    If     MeaningfulName77 = MeaningfulName1 Then 'Curve 1
                        c = 10
                    ElseIf MeaningfulName83 = MeaningfulName1 Then 'Curve 2
                        c = 11
                    ElseIf MeaningfulName89 = MeaningfulName1 Then 'Curve 3
                        c = 12
                    ElseIf MeaningfulName95 = MeaningfulName1 Then 'Curve 4
                        c = 13
                    ElseIf MeaningfulName101= MeaningfulName1 Then 'Curve 5
                        c = 14
                    End If
                    If c > -1 Then
                        For x = 8 To UBound(ArrForwardCurves, 1)
                            If MeaningfulName158 = ArrForwardCurves(x, 1) 
                            Then 'Populate curve c Price
                                PricingDatesArr(j, c) = ArrForwardCurves(x, n)
                            End If
                        Next x
                    End If
                Next n
            Else ' not forward fixed price
                …

for a handful of meaningful names?

\$\endgroup\$
3
  • \$\begingroup\$ I don't read VBA. \$\endgroup\$ Commented May 2, 2022 at 7:36
  • \$\begingroup\$ Thanks for your input, I wish I didnt have to read it. \$\endgroup\$ Commented May 3, 2022 at 14:11
  • \$\begingroup\$ You can combine "both non-fixed" cases into the above - I found it too unsettling without meaningful names before&after. \$\endgroup\$ Commented May 3, 2022 at 14:47

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.