0

I am working as a Data Analyst for a software startup where I am required to write macros to analyse and sort data more efficiently. I am currently working on a macro that takes a value one workbook ("Job MMRF") and searches for it in another ("U100 Material Information"). My code is as follows:

Sub MMRFValidation()

Dim c As Range
Dim leadtime As Double
Dim price As Double

Application.ScreenUpdating = False

With Workbooks("Job MMRF.csv")
    For Each c In Range("C:C")
        If c.Value = "" Then
            c.Offset(, -2).Font.Color = vbRed
            c.Offset(, 9).Value = "Need to contact vendor"
            c.Offset(, 10).Value = "Need to contact vendor"
        Else

            Dim a As Range

            With Workbooks("U100 Material Information.xlsx")
                For Each a In Range("A:A")
                    If a.Value = c.Value Then
                        price = a.Offset(, 15).Value
                        leadtime = a.Offset(, 13).Value
                    End If
                Next a
            End With

            If price = 0.01 And leadtime = 21 Then
                c.Offset(, -2).Font.ColorIndex = 7
                c.Offset(, 9).Value = leadtime
                c.Offset(, 10).Value = price
            Else
                c.Offset(, -2).Font.Color = vbGreen
                c.Offset(, 9).Value = leadtime
                c.Offset(, 10).Value = price
            End If
        End If
    Next c
End With

Application.ScreenUpdating = True

End Sub

c is the value from the first workbook. I am trying to find c in the second workbook. If it is found, I want to copy the values from the 13th and 15th column in the U100 wb (associated with the row where c was found) and paste these values into the 9th and 10th row in JobMMRF (associated with the row where c initially was). The part of the code that changes font color works, but the price/lead time part does not. Pls help, thanks.

edit: I have updated the code. Now it pulls price and leadtime values, but for some reason they are always 0.

Sample data enter image description here

Ricardo Diaz
  • 5,658
  • 2
  • 19
  • 30
mcar333
  • 3
  • 3
  • 2
    Can you be specific? "Does not [work]" isn't very helpful. – BigBen Feb 28 '20 at 15:12
  • By does not work I mean that no value is retrieved, the column where the pricing and the lead time should be is blank. – mcar333 Feb 28 '20 at 15:14
  • 1
    Btw - why are you working with `String`s for `price` and `leadtime`? That doesn't seem right. – BigBen Feb 28 '20 at 15:16
  • I am kind of a beginner in terms of VBA, so it is just something we kind of did lol. What would you recommend we use instead ? – mcar333 Feb 28 '20 at 15:18
  • Numbers? Surprised your code compiles with `Next Entry`? – SJR Feb 28 '20 at 15:19
  • `String` = text. You want a numeric type. See [this](https://learn.microsoft.com/en-us/office/vba/language/reference/user-interface-help/data-type-summary). – BigBen Feb 28 '20 at 15:19
  • i have editted the code - changed price and leadtime to Double and fixed Next Entry to Next a. Still experiencing the same issues – mcar333 Feb 28 '20 at 15:30
  • Remove the quotes from "0.01" and "21" too. – BigBen Feb 28 '20 at 15:33
  • 1
    And stop activating things and refer properly to ranges https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba – SJR Feb 28 '20 at 15:34
  • @SJR activating what? The workbooks or actual cells? – mcar333 Feb 28 '20 at 15:38
  • Avoid activating anything as far as possible (in Excel) - read the article and all will be revealed. – SJR Feb 28 '20 at 15:42
  • Btw you could avoid your inner loop using FIND or MATCH. – SJR Feb 28 '20 at 15:43
  • @SJR could you provide an example? – mcar333 Feb 28 '20 at 15:46
  • @SJR also there is nothing in the article about inactive workbooks – mcar333 Feb 28 '20 at 15:51
  • It explains how you can directly refer to workbooks, worksheets and ranges without having to select or activate them first. Read it until it goes in because it will move your coding onto another level. – SJR Feb 28 '20 at 16:02
  • Is this the same question as [here](https://stackoverflow.com/questions/60439572) ? – CDP1802 Feb 28 '20 at 16:21
  • @CDP1802 yes cccccccc – mcar333 Feb 28 '20 at 16:38
  • ive updated the code. Please reread description for new problem – mcar333 Feb 28 '20 at 16:40
  • If that is the code you are using I can't understand why For Each c In Range("C:C") doesn't fill all the empty rows down to 1,048,576. – CDP1802 Feb 28 '20 at 18:04

1 Answers1

0

I've refactored your code.

Read the comments and adjust it to fit your needs

EDIT: I have modified the way it's handled the cases where the csv cell value is different than an empty string.

Code:

Sub MMRFValidation()

    Dim csvWorkbook As Workbook
    Dim csvSheet As Worksheet
    Dim csvRange As Range
    Dim csvcell As Range

    Dim csvLastRow As Long

    Dim materialWorkbook As Workbook
    Dim materialSheet As Worksheet
    Dim materialRange As Range
    Dim materialCell As Range

    Dim materialLastRow As Long

    Dim leadtime As Double
    Dim price As Double

    Application.ScreenUpdating = False

    ' Adjust workbook and worksheet names
    Set csvWorkbook = Workbooks("Job MMRF.csv")

    Set csvSheet = csvWorkbook.Worksheets("CSV SHEET NAME") ' <- ADJUST SHEET NAME

    Set materialWorkbook = Workbooks("U100 Material Information.xlsx")

    Set materialSheet = materialWorkbook.Worksheets("MATERIAL SHEET NAME") ' <- ADJUST SHEET NAME

    ' This looks for the last row in column C
    csvLastRow = csvSheet.Cells(csvSheet.Rows.Count, "C").End(xlUp).Row

    ' This looks for the last row in column A
    materialLastRow = materialSheet.Cells(materialSheet.Rows.Count, "A").End(xlUp).Row

    ' Set the range from C1 to last row
    Set csvRange = csvSheet.Range("C1:C" & csvLastRow)
    Set materialRange = materialSheet.Range("A1:A" & materialLastRow)

    ' Loop through each cell in target range
    For Each csvcell In csvRange.Cells

        ' If no value
        Select Case True
        Case Trim(csvcell.Value) = vbNullString
            csvcell.Offset(, -2).Font.Color = vbRed
            csvcell.Offset(, 9).Value = "Need to contact vendor"
            csvcell.Offset(, 10).Value = "Need to contact vendor"

            ' Reset if null?
            price = 0
            leadtime = 0
        Case Else
            ' Get matching cell in material workbook
            Set materialCell = GetMatchedCell(materialRange, csvcell.Value)

            ' If found
            If Not materialCell Is Nothing Then
                price = materialCell.Offset(, 15).Value
                leadtime = materialCell.Offset(, 13).Value
            Else
            ' Reset if not?
                price = 0
                leadtime = 0
            End If

            ' Moved this to only the cases where the csvcell value is different than null
            With csvcell

                If price = 0.01 And leadtime = 21 Then
                    .Offset(, -2).Font.ColorIndex = 7
                    .Offset(, 9).Value = leadtime
                    .Offset(, 10).Value = price
                Else
                    .Offset(, -2).Font.Color = vbGreen
                    .Offset(, 9).Value = leadtime
                    .Offset(, 10).Value = price
                End If

            End With

        End Select

    Next csvcell

    Application.ScreenUpdating = True

End Sub

Private Function GetMatchedCell(ByVal lookupRange As Range, ByVal lookupValue As Variant) As Range

    Dim lookupCell As Range

    For Each lookupCell In lookupRange.Cells

        If lookupCell.Value = lookupValue Then
            Set GetMatchedCell = lookupCell
            Exit For
        End If

    Next lookupCell

End Function

Let me know if it works

Ricardo Diaz
  • 5,658
  • 2
  • 19
  • 30
  • Thank you so much Ricardo!! You're an absolute beauty – mcar333 Mar 02 '20 at 14:12
  • For some reason the code is pulling a price and leadtime value for empty csvcell.Values. To me that indicates there is something wrong with the following portion of code: # Select Case True Case csvcell.Value = vbNullString csvcell.Offset(, -2).Font.Color = vbRed csvcell.Offset(, 9).Value = "Need to contact vendor" csvcell.Offset(, 10).Value = "Need to contact vendor" # however i cant figure out why – mcar333 Mar 02 '20 at 14:52
  • `If cell value is equal to "" then do something` maybe the cell is not empty rather has a zero in it? You can step through the code pressing `F8` key and hover over portions of the code to see (in this case over csvcell.value) what value is holding in each step – Ricardo Diaz Mar 02 '20 at 15:31
  • Ive tried changing vbNullString to "" and " ". I've also tried making it search for the value in the A column (Items with an emty C-column cell will have "Add" in the A-column). For some reason it keeps pulling random values for price and lead time and changes the text in column A to green instead of red. Ive debugged the macro but still cant determine what value it is pulling since the C-column cell is empty – mcar333 Mar 02 '20 at 15:46
  • `vbnullstring`is equivalent to `""` so what I was suggesting was that maybe those cells have `0` instead of `""` – Ricardo Diaz Mar 02 '20 at 15:54
  • I posted a pic of the excel doc as an answer to this question to clarify- tried changing vbNullString to 0 but Im still experiencing the same problem – mcar333 Mar 02 '20 at 16:06
  • Please remove your answer and use the [edit] button to add the picture to your original question. I'm gonna look into it and get back to you. – Ricardo Diaz Mar 02 '20 at 16:10
  • Side note: My bad I used the edit button of my answer. Should have used your question edit button. Still looking into it... – Ricardo Diaz Mar 02 '20 at 16:26
  • I have adjusted the code based on your image. Let me know if now it's working as expected – Ricardo Diaz Mar 02 '20 at 16:32
  • Awesome. Remember to mark the answer with the check mark at the left so others may find it too – Ricardo Diaz Mar 02 '20 at 16:42