0

I have coded a macro that appears to work (on some worksheets), but it seems rather heavy to me. Can you please take a look and see if changes can be made or a completely different macro that completes the same task. I am looking for a value "EUR" or "USD" in a number of specified columns. When either of the two values are found, the macro performs a calculation on adjacent cells and then changes the value of the original cell to AED.

Option Explicit

Sub Change_currency()
Const EUR_to_AED = 4.9
Const USD_to_AED = 3.64
  Dim R As Long
  Dim V1
  Dim V2
  Dim MyValue
  Dim MyValue2

  'Start at row 5
  R = 5

  While Cells(R, "K").Value <> ""
    If Cells(R, "K").Value = "EUR" Then
        Cells(R, "K").Activate
        MyValue = 4.9
        V1 = ActiveCell.Offset(0, -2).Value
        V2 = ActiveCell.Offset(0, -3).Value
        ActiveCell.Offset(0, -2).Value = MyValue * V1
        ActiveCell.Offset(0, -1).Value = V1 * EUR_to_AED * V2
        ActiveCell.Value = "AED"

    End If

While Cells(R, "K").Value <> ""
    If Cells(R, "K").Value = "USD" Then
        Cells(R, "K").Activate
        MyValue = 3.64
        V1 = ActiveCell.Offset(0, -2).Value
        V2 = ActiveCell.Offset(0, -3).Value
        ActiveCell.Offset(0, -2).Value = MyValue2 * V1
        ActiveCell.Offset(0, -1).Value = V1 * USD_to_AED * V2
        ActiveCell.Value = "AED"

End If

    'Next row
    R = R + 1
  Wend

End Sub

I am running into problems with some of the data on the sheets are not numerical and the macro throws an error. So how to ignore those errors?

Any help would be appreciated...

pnuts
  • 58,317
  • 11
  • 87
  • 139
  • Please avoid using Activecell and Activate. You might want to see [THIS](http://stackoverflow.com/questions/10714251/excel-macro-avoiding-using-select/10718179#10718179) – Siddharth Rout Sep 16 '13 at 14:30
  • Also before performing the calculations, why nnot check if the cell has numeric values? – Siddharth Rout Sep 16 '13 at 14:32
  • Thanks Siddharth - Well above me head though - hence me asking some questions here... – totalpackage Sep 16 '13 at 14:39
  • 1
    Well you have to start somewhere :) Try incorporating the tips given in that link and like I mentioned, validate the cell values and check if they are numeric or not :) – Siddharth Rout Sep 16 '13 at 14:49

1 Answers1

2

I usually prefer a For...Next loop to WhileWend, and you seem to be doing the exact same thing twice, depending on the currency, so I would simplify it:

Sub Change_currency()

    Const EUR_to_AED          As Double = 4.9
    Const USD_to_AED          As Double = 3.64

    Dim wks                   As Worksheet
    Dim lngRow                As Long
    Dim lngNumRows            As Long
    Dim c                     As Range
    Dim dblConversion         As Double

    Dim V1                    As String
    Dim V2                    As String

    Set wks = ActiveSheet

    With wks

        ' this gets the last row number in column 11 that has a value
        lngNumRows = .Cells(.Cells.Rows.Count, 11).End(xlUp).Row

        For lngRow = 5 To lngNumRows

            Set c = .Cells(lngRow, 11)

            Select Case c.Value
                Case "EUR"
                    dblConversion = EUR_to_AED
                Case "USD"
                    dblConversion = USD_to_AED
            End Select

            V1 = c.Offset(0, -2).Value
            V2 = c.Offset(0, -3).Value

            If IsNumeric(V1) And IsNumeric(V2) Then
                c.Offset(0, -2).Value = V1 * dblConversion
                c.Offset(0, -1).Value = V1 * dblConversion * V2
                c.Value = "AED"
            End If

        Next lngRow

    End With

End Sub
dendarii
  • 2,958
  • 20
  • 15