PDA

View Full Version : Help! VBA codiing is not my expertise...



dularmer
12-14-2023, 01:41 PM
Hi all,

I am hoping to find someone who would be willing to review my code and see how I can fix my tiny issue.
I am currently trying to have this vba code calculate 'Old PVO' using 'Previous year APP' as the prevailing price * Annual forecast total (Annual forecast FYXX + Annual forecast FYXY). This also works perfectly fine when there's actually data in the 'Previous year APP' cell, however I had written into the code that it should be taking 'Current MAP' if 'Previous year APP' is empty, but it simply does not work regardless of what I try.

Everything else the vba code does seems to work - it's 'only' this one thing it simply won't do.

Any suggestions?


Sub CalculateSavingsInRange()
Dim i As Long
' Set your worksheet password
Dim wsPassword As String
wsPassword = "Password"
' Unlock the worksheet
ThisWorkbook.Sheets("BC calculation").Unprotect Password:=wsPassword ' Replace "Sheet1" with your sheet name
For i = 6 To 15 ' Loop through rows 6 to 15
' Initialize all variables
Dim annualForecastFYXX As Double
Dim annualForecastFYXY As Double
Dim previousYearApp As Variant
Dim currentMap As Variant
Dim oldPVO As Double
Dim newPrice As Double
Dim newPVO As Double
' Column E: FYXX
If Not IsEmpty(Cells(i, 5).Value) And IsNumeric(Cells(i, 5).Value) Then
annualForecastFYXX = CDbl(Cells(i, 5).Value)
Else
annualForecastFYXX = 0 ' Or any other appropriate value
End If
' Column G: FYXY
If Not IsEmpty(Cells(i, 7).Value) And IsNumeric(Cells(i, 7).Value) Then
annualForecastFYXY = CDbl(Cells(i, 7).Value)
Else
annualForecastFYXY = 0 ' Or any other appropriate value
End If
' Column H: Previous Year App
If Not IsEmpty(Cells(i, 8).Value) And IsNumeric(Cells(i, 8).Value) Then
previousYearApp = CDbl(Cells(i, 8).Value)
Else
previousYearApp = 0 ' Or any other appropriate value
End If
' Column I: Current Map
If Not IsEmpty(Cells(i, 9).Value) And IsNumeric(Cells(i, 9).Value) Then
currentMap = CDbl(Cells(i, 9).Value)
Else
currentMap = 0 ' Or any other appropriate value
End If
' Column J: New Price
If Not IsEmpty(Cells(i, 10).Value) And IsNumeric(Cells(i, 10).Value) Then
newPrice = CDbl(Cells(i, 10).Value)
Else
newPrice = 0 ' Or any other appropriate value
End If
' Calculate the Old PVO
Dim annualForecastTotal As Double
' Default value if neither previousYearApp nor currentMap is numeric or empty
oldPVO = 0
' Check if Column H contains a numeric value or is empty
If Not IsEmpty(previousYearApp) And IsNumeric(previousYearApp) Then
annualForecastTotal = annualForecastFYXX + annualForecastFYXY
oldPVO = annualForecastTotal * CDbl(previousYearApp) ' Use Column H
ElseIf Not IsEmpty(currentMap) And IsNumeric(currentMap) Then
annualForecastTotal = annualForecastFYXX + annualForecastFYXY
oldPVO = annualForecastTotal * CDbl(currentMap) ' Use Column I
End If
' Place the calculated value in Column L only if there's an actual value to calculate
If oldPVO <> 0 Then
Cells(i, 12).Value = oldPVO
End If
' Debugging: Display values for troubleshooting
Debug.Print "previousYearApp: " & previousYearApp
Debug.Print "currentMap: " & currentMap
' Calculate the New PVO
' Check if Column J contains a numeric value or is empty
If Not IsEmpty(newPrice) Then
If IsNumeric(newPrice) Then
annualForecastTotal = annualForecastFYXX + annualForecastFYXY
newPVO = annualForecastTotal * newPrice ' Use Column J
Else
newPVO = 0 ' Cell is not numeric
End If
Else
newPVO = 0 ' Column J is empty
End If
' Place the calculated New PVO in Column M only if there's an actual value to calculate
If newPVO <> 0 Then
Cells(i, 13).Value = newPVO
End If
' Calculate savings for FYXX
CalculateAndPlaceSavings i, 12, 13, 14, 4
' Calculate savings for FYXY
CalculateAndPlaceSavings i, 12, 13, 15, 6
Next i
' Lock the worksheet again with the same password
ThisWorkbook.Sheets("BC calculation").Protect Password:=wsPassword
End Sub


Sub CalculateAndPlaceSavings(ByVal row As Long, ByVal oldPVOColumn As Long, ByVal newPVOColumn As Long, ByVal savingsColumn As Long, ByVal criteriaColumn As Long)
' Exit the subroutine if the row exceeds 15
If row > 15 Then Exit Sub
Dim oldPVO As Double
Dim newPVO As Double
Dim savings As Double
' Check if the cells contain numeric values and are not empty before assigning to Double variables
If IsNumeric(Cells(row, oldPVOColumn).Value) And Not IsEmpty(Cells(row, oldPVOColumn)) And _
IsNumeric(Cells(row, newPVOColumn).Value) And Not IsEmpty(Cells(row, newPVOColumn)) Then
oldPVO = CDbl(Cells(row, oldPVOColumn).Value)
newPVO = CDbl(Cells(row, newPVOColumn).Value)
' Calculate savings based on your criteria
savings = ((oldPVO - newPVO) / 12) * Cells(row, criteriaColumn).Value
Else
savings = 0
End If
' Place the calculated savings in the specified column only if there's an actual value to calculate
If savings <> 0 Then
Cells(row, savingsColumn).Value = savings
End If
End Sub

Paul_Hossler
12-14-2023, 02:55 PM
Don't know if this is the issue, but "Cells" without an explicit worksheet will assume what ever the active sheet is



If Not IsEmpty(Cells(i, 5).Value) And IsNumeric(Cells(i, 5).Value) Then
annualForecastFYXX = CDbl(Cells(i, 5).Value)
Else
annualForecastFYXX = 0 ' Or any other appropriate value
End If


Other than that, without an attachment with some data to test with, it's hard to tell

dularmer
12-14-2023, 03:27 PM
File attached. Hope that helps31264

June7
12-14-2023, 04:31 PM
"I had written into the code that it should be taking 'Current MAP' if 'Previous year APP' is empty"

What line do you expect to accomplish that?


' Check if Column H contains a numeric value or is empty
If Not IsEmpty(previousYearApp) And IsNumeric(previousYearApp) Then
...
ElseIf Not IsEmpty(currentMap) And IsNumeric(currentMap) Then
...
End If

These are VBA variables, not cells. Code has already populated with cell value or 0 so they will not be empty or null.

Use IsNull() or = 0 or = "" depending on variable type. Since these Variant variables will not be Null, perhaps test for =0.

Why don't you merge with earlier code that tests if H and I cells are empty? Or eliminate If Then Else in this case:


annualForecastTotal = annualForecastFYXX + annualForecastFYXY
oldPVO = annualForecastTotal * CDbl(IIf(previousYearApp <> 0, previousYearApp, currentMap))

oldPVO, newPVO and newPrice are declared as Double type and therefore can never be empty or Null - default value is 0. Only Variant type can hold Null. Number types will default to 0 and String defaults to empty string and Date type defaults to 12:00:00 AM.